Lately, I’ve been going back through some of my old gems and libraries to modernize them. It’s been very interesting to see how my coding style and design has changed over the years!
I’d love some feedback about the gem that I’ve been updating most recently - sub_diff. I’ve gotten to the point where I don’t feel like I can simplify or refactor the logic down any further. I think it embodies what I consider to be “beautiful” and “clean” code at this point in my career. I’ve applied many of the design principles and practices that I’ve picked up from upcase and thoughtbot, so thank you guys very much for your guidance!
It’s pretty interesting to compare it to the old version of sub_diff that I wrote back in 2011. Back then I seemed to favor squeezing in as much functionality as possible into one line at an attempt to keep my methods short. Since then my style has definitely changed, resulting in code that focuses more on clarity and conciseness. Each line of code is designed to be as simple as possible which is pretty different than the way I used to do it!
I’m sure there’s still things that can be improved in the codebase so I’m hoping some fresh pairs of eyes will help point them out!
I’m curious if you guys think Builder has too many responsibilities. It’s the only class that knows about the other private classes, but it also knows how to use them. I feel like accepting the private classes as dependencies might be a little bit of an overkill in this case. Maybe there’s some type of Factory object that can be extracted out? Do you guys think that would make things any clearer?
I’m not convinced moving to a factory makes this clearer. I had trouble following the code in the pull, personally. (Take with a grain of salt. Trying to drop in to an existing project will usually be confusing.)
Just scanning Builder, I’d hesitate to say it’s doing too much. Line-count is an imperfect measure of complexity, but when I look at this class, it looks “small enough” that I probably wouldn’t worry about it.