Database Migrations Converting DbScriptomate to Ruby - Fixing Something I Don't Like
Yesterday we saw this code that will run the SQL scripts that are passed to it:
I don’t like it! The method signature looks as if it will only grow and why are there readable attributes set, who is going to need the SQL Server specific username, password, host and database?
If we look at the creation of the new TinyTDS Client class, we get a hint on how to clean this code up. If we were to pass in an options Hash we could make our life a lot easier, so let’s do that.
In this code, we are passing in an options Hash, and this is used in the call to our TinyTDS class, it just looks better. The initialize method does not look a bunch better, but it’s getting there.
Let’s make another change. I said earlier that we no longer needed the readable attributes so let’s get rid of that code.
That feels much better; there is a lot less noise going on in this class. Now the SqlServerRunner is just a thin wrapper over TinyTDS.
So how does this affect our Command class? Well, we can simplify the execute method that is part of the Command class and make it a lot easier on the lines. Here is how the code looks before we refactor it.
Now after the refactor the code looks better.
We only got rid of a few lines of code but the good thing is our code is a little better. Something a lot of developers tend to forget is that refactoring doesn’t need to be an exercise in mimicking a tornado, you don’t need to come in and drastically change everything. Small, thoughtful changes will do the job. Your team will be much happier with small incremental changes and the guy that does a peer / code review will also be happy that he isn’t confronted by a wall of red and green when they open your pull request.