Custom Source Control: Refactoring
This is a continuation of my last post on implementing a “custom source control” application in ruby. In that post we built a custom source control application based on “Source Control Made Easy” by Jim Weirich.
Before starting you may want to checkout the repository for the application on github. You can then checkout the part-1-complete release. Or if you’d rather just down a zip file containing the source for custom source control: part-1-complete. Let’s refactor…
Recall our initial stories from the previous post:
I also left you with a list of some stuff I wanted to address in future posts. Lets take another look at what I will be working on now:
- Separating the tests from the actual implementation code.
- DRY’ing out our code.
So without further ado…
Separating the tests from the actual implementation code.
The first thing that we’ll want to do is separate the tests from the application code. Start out by changing directories into the custom_source_control directory.
Next lets create a directory for our code and another for our tests (which going forward I may refer to them as specs).
Our directory structure should now look like the following:
Now we’re going to create a Rake file. Rake is a build system that we will use from amongst other things, running our specs. I’d like to mention that Rake is another tool brought to us by Jim Weirich.
So what are we doing here? Basically we are making it easy to run our specs. So we’ll be able to do: rake test
for running our tests, and something else to run the application. We’re now going to create a spec helper file that will require our application code as well as the minitest library.
Now let’s move everything in lib/custom_source_control.rb
from the require 'minitest/autotest'
line down, to a new file specs/custom_source_control_spec.rb
. At the top of the file we need to change the require 'minitest/autotest'
to require 'helper'
. We also need to change the before
block so that as part of the testing, we change directory to the test_dir
directory.
At this point we should be able to run our tests with rake test
, and everything should be passing.
DRY’ing out our code.
Next up on our list… DRY or Don’t repeat yourself, Basically where we find we have code doing a very similar job is a potential candidate. Doing a visual scan of the code and I notice a few control_file_exists?
methods. These methods serve as a convenient way to check for the existence of some of the mandatory repository files. One way to DRY this functionality up is to use the define_method
method.
So I start out by refactoring the manifest_exists?
and metadata_exists?
methods into:
What we’re doing is creating an array taking the prefix of previous methods and iterating through that array. With each element we’re passing it in as an argument to the define_method
method. The method body should be pretty straightforward; we’re doing a File.join
on the repository directory, and the control files. Let’s run our tests to make sure everything still works.
Why stop there? Let’s add head_exists?
.
So in refactoring head_exists?
we had to turn the array of method names into an array of filenames. This would create methods __manifest___exists?
, __metadata___exists?
& HEAD_exists?
. Which is not what we want. So we simply add a call to the String#downcase
method which properly creates the head_exists?
method. We also add a call to String.gsub
to handle removing the _
’s. The last thing is to change the File.join
and remove the _
’s as well.
(retest)
The next bit of code has to do with the cwd_hashes
method.
It instantiates a new SHA1 object, then iterates through the current working directories files and hashes them, assigning the hash to… well our hashes hash. I am tempted to refactor the code to something like the following:
Which passes our tests, but I wonder if we take a performance hit since we’re now creating multiple instances of a SHA1 object. It would be nice if we could benchmark this somehow… Well we can! First let’s add the following code:
Add the following to to the bottom of the file:
Everything wrapping the benchmark stuff is the same setup code we’re using in our tests.
Next let’s add the following to our rake file:
Run it with:
It should output results similar too:
As you can see, not much of a difference.
The next pieces of code are the hash_and_copy_manifest
& hash_and_copy_metadata
methods.
(retest)
There is not a whole lot going on in the initialize_repository
method, but in snapshot
we can make a few changes.
The creation of the metadata & manifest files doesn’t need to happen because we do a File.open
and write to them within the write_manifest
& write_metadata
methods.
The next thing to fix is the fact that metadata timestamp was hardcoded to 2014-03-07 23:59:59 -0800
. We can remove that line of code, but the tests will fail again. So what can we do? Stub the Time.now
method. If you’re not on ruby 2 or higher, you are going to need to take a few extra steps. Create a Gemfile
file and add the following:
Now we update helper.rb
with the following:
in custom_source_control.rb
:
in custom_source_control_spec.rb
, wrap the stuff inside our before
blocks in:
(retest)
I left off in the last post with a question: How do I use this thing now that it’s built?. I left off with a suggestion on how to make the script runable. Now I’d like to update the script and add this functionality. Open up custom_source_control.rb
and add the following to the very bottom.
Lastly, what I’ve done is symlink the file into my /usr/local/bin
directory as csc
Still, more to do:
- We don’t really clean up after ourselves so that functionality needs to be added.
- Getting the checkout hash is also a manual process so that
csc log
functionality we talked about would come in handy. - We are not handling any types of errors mind you
- Testing for more edge cases, and fixing any bugs we find.
- Adding code coverage.
- Adding the ability to handle command line arguments with
OptionParser
. - Adding tests and functionality to diff checkins.
- Adding tests and functionality to list the history (metadata file hashes from head all the way back to root)
- Possibly turning this into a gem.
…so, again, its not quite production ready.
You can double check your work with the project I have hosted on github