2015-05-04.md

Notes on britneywright/exercism_json

The code options[:repo_name].split(","), in call_contributors is parsing command-line arguments. We'd like this method to be callable from anywhere in our app, so we want to push things like dealing with commandline arguments up higher. In this case, we'd push it all the way up to parse, on line 9. We probably need to do argv[2].to_s.split(","), so that it works correctly whether there are arguments or not.

nil.to_s.split(',')            # => []
'repo1'.to_s.split(',')        # => ["repo1"]
'repo1,repo2'.to_s.split(',')  # => ["repo1", "repo2"]

We'd want to make the option name plural, since it's now an array. And so the check on line 15, argv[2] == nil would become options[:repo_names].empty?, which allows us to think in terms of the names, making it easier to reason about.

This means that now, down in our call_contributors method, we don't need to have two different code paths. The case where there is only one repo name is now the same as having multiple repo names, in both cases, we'll just iterate over the repo names, however many there happen to be, and perform the appropriate job.

To handle some of the complexity of the code in contributors, we can push it down into methods that contain that complexity, and give a name to it, so that we can think in more abstract terms. For example, we have the line number_of_pages = first_page.headers["link"].split(",")[1].match(/&page=(\d+)/)[1].to_i This could be changed to be something like number_of_pages = num_pages(first_page.headers["link"]) Which we would push that code down into:

def self.num_pages(link_header)
  link_header.split(",")[1].match(/&page=(\d+)/)[1].to_i
end

It is possible that there could be several methods extracted from contributors, and we might also do a similar thing for one of the other methods. If this happens, it could be there are sets of methods all on our Github::Json object, that are called together, and independent of each other. For example, we can see that contributors_url_for is only called from this method, so we would have at least two methods pulled out of this. If this seems to be true, that's a good sign that we should separate them, so that we can take each idea independently, and prevent its changes from messing with the rest of the code. A common way to do this is to make an object, though, if it seems pointless to have an object (ie we don't find ourselves feeling a need to store instance variables) then we could make them all singleton methods, like we currently have, but just placed on a different object than Json.

Here is how that could wind up looking. Currently we have response = contributors(options). If we made it into an object, it might look like response = Contributors.new(options).call, which would then imply a renaming like contributors = GetContributors.new(options).call. And if we did something like that, and decided there was no real value in making it an object, we could do it entirely in class methods like this: contributors = GetContributors.call(options)