TLDR; Keeping with the refactoring stories started with My GPA in Code Climate is 3.59: A Refactoring Story I decided today to get a piece of code as much in shape as my knowledge let me, of course I’m always waiting for suggestions on how to improve so please do leave comments.
This code base is part of an exercise from the thoughtbot learning program Learn Prime which have great content go ahead and check it out you won’t regret it. So getting that out of the way, let’s see the code:
Surprise; that looks like a pretty solid report. Following my own rule regarding 10 or less on flog which is good. Which means that I’m done, right? Of course not this is a refactoring post so I have to at least rename a variable. Let’s use another tool and see what it says this time I’m going to use Reek.
Surprise again; so there’s no warnings from reek regarding any code smell. So I think now I’m done, right? Well again I need to do something with this code. But I will stop just using tools I will just create some constraints for myself and try to shape the code on that; the constraints are basically this list: Object Calisthenics
I don’t have any abbreviations at the moment on this particular piece of code and most name make sense to me. Probably I’m wrong but that’s my perception at the moment. Btw this constraint mean not to shrink method names, variables and constants.
Keep All Entities Small
So the rule says that no class should contain more than 50 lines and no package should contain more than 100 files. Both things are taking care of now; the only thing that I did at the end was to move the classes to it’s own files.
No Classes With More Than Two Instance Variables
This was kinda hard; but it just points me to the fact that I did not follow rule number 3 properly; because not every primitive is wrapped. The advantage of this rule is that force you to encapsulate and distinguish classes that do orchestration which does that work as containers. Which reminds me the distintion in the Book Growing Object Oriented Design Guided by Tests between objects and values. Of course the number of instance variables is arbitrary but I think is a good number that help me stretch my brain a little bit ;–)
This particular rule in fact refers to Tell Don’t Ask which means that is fine to use accessors to know an object’s inside state but not to make any decisions with that data outside the object. So basically I just remove the attr_accessors that I had in the SurveyInviter class just because I don’t need them any more and that will expose data that I really don’t need to use outside of the class.
So there you have it; another happy ending for a refactoring story; I learned a lot from this one, hope I shared as much. Enjoy.