Day 9 - Slim down an overgrown class

Difficult to make a lot of progress in 20 minutes but did help me identify several classes we need to slim down and/or improve.

All of the current project classes are under 100 lines. I reviewed them and found a great candidate for extraction: Creating a CSV file from model objects. But the thing is too tightly coupled, so I added that item to the future refactoring list.

Going through the other activities, I found a comment. We have a number of comments that explain why a method exists — in this case some random bad (unexpected?) behavior of a framework, usually with a link to the full explanation. This comment said only “See Gotcha.” I’m guessing there used to be more to the comment? I didn’t go looking. I understood the gotcha, found the documentation link and added it. This code quality will definitely help someone in the future. Yep, probably me.

1 Like

We have one very large react component that’s been refactored once recently (consolidating some component organization that didn’t make sense), and is still sitting at an extremely hefty 300 lines or so. It’s a mix of too many components and a lot of logic. It’s also undergoing a lot of churn as part of a new feature, so we’re hoping it can slim down as we make those changes.

On the other hand, we are tracking some older Ruby classes that are also quite large. Our team uses codeclimate and we always get warnings that it’s many, many methods over what is considered a healthy amount of methods in a class. Our strategy is to slowly chip away at it every time we make changes and that file needs to be touched.

1 Like

we had one class sitting around with over 2k zip codes hard coded within it. one of those “we need to ship immediately, so we’ll come back and fix it later” kind of decisions. it was part of a larger refactoring, but I was finally able to get rid off over 400 lines of code that was just zip codes :raised_hands:

1 Like

My User class was 767 lines big. Two methods showed promise to be moved to anther class, but since the unit test fixtures for those methods were tricky the job looked too daunting to pull off in 20 minutes. Ended up removing uneeded comments and adding a docblock to an undocumented method instead.

I’ve that find command as a function called godClasses on my scripts collection :smiley:
Quite scary what numbers one gets returned when applied to projects that have been in production for a while. Found two projects where the “winner” class had 4 digit line number count. Interestingly, in my case always just one class and then the 2nd place has already less than 50% lines compared to the 1st.

I’ve tried several things in this time, but finally I did just some minor improvements.
Nice one - improving my mind for my next project :nerd_face:

3 files changed, 2 insertions(+), 58 deletions(-)

Most of it was dead code, but anyway. A tiny improvement for a class with 3426 LOC :frowning:

I added a god file instead of removing one today :woozy_face:

Couldn’t get bundling/transpilation of Rollup + Babel + CoffeeScript working in my side-project so I just concatenated the source files together to simplify the module resolution aspect of things. It’s still sub-200 LOC so it’s not totally unmanageable and will allow me to move forward on other things, most importantly publishing an NPM package so I can start using this elsewhere, which will lead to an overall reduction in code.

Hi there! Removed a bunch of unused comments (and also commented code) today. Feels good!

I found a 1,000 line file. I was shocked! I tried to extract 10+ nearly identical methods with method_missing but I couldn’t get the test to pass on time. I guess I have to throw this big fish back in the pond until next month.

I’m quite strict with the time limit because one could easily spend hours cleaning up the code. At the very least, I still get to explore new areas of the code. I also noted that this file will have to be updated in the near future with a big project that we’re working on.

I found in the biggest model a method that is called only in tests, today only had the opportunity to ensure is code no longer used in production and opened a ticket, tomorrow I will remove it

find ~/code/my_project -name "*.rb" | xargs wc -l | sort -rn | head
I spent half of my 20 minutes picking the shell command apart and adding a few bits to my command line tool kit :slight_smile:

1 Like

Nice tips!
Could improve some classes on my codebase <3

Like others, I spent a good amount of time tweaking the shell command. My first attempt was fd, but it turns out globs are a little limited. In the end I used ls -1 app/**/*.rb | xargs wc -l | sort -rn | head instead (with zsh). I was able to exclude spec files that way.

The classes I found were usual suspects. I did not mount a full extract refactoring, but did some minor, yet sweet tweaks. Also learned (or was reminded of?) the fact you can list expressions in a Ruby case/then statement.

Finally came along a TODO, fixed it and may have improved performance a little.

Not wildly satisfying. I did find some TODO’s (not purged from here because I was working on a different project for the todo day) which I ripped out with glee. So many commented out functions and lines of code died this day as well.

Catching up from last week as I was in the middle of getting a feature finished on an open branch.

I refactored a few methods on my longest class, although most of the class is database field declarations :man_shrugging: so I am not about to start migrations in a 20 minute slot :smiley: