Refactorings welcome?

[origino...@gmail.com]

Howdy, Calagator! My name is Micah Geisel, Portlander and Rubyist.

I am currently mentoring a few students, and I'd like them to spend some time refactoring an open-source Rails application to get a feel for refactoring in general, working with a real-world Rails app, and collaborating on an open source project. Living in PDX, our first thought was Calagator. After taking a look through the code, I see some opportunities:

Smaller ideas:

1. Add coveralls.io for coverage reporting and nifty readme badge. This will probably expose some areas where we can improve test coverage.

2. Refactor to more idiomatic Ruby. Lots of explicit returns, unnecessary usage of `self.`, for loops, etc. Rubocop can probably provide guidance here.

3. Tons of minor refactorings to existing methods. Preferring `super` to `alias_method_chain`, leveraging Enumerable instead of building arrays manually, replacing manual type checking with polymorphism, etc.

Medium ideas:

3. Event appears to be a god object. Plenty of opportunities for object extraction, particularly at the class method level.

4. Code Climate has highlighted many other opportunities for refactoring, as well.

5. Write acceptance tests for happy path workflows. Things are pretty well tested on the unit-ish level, and some of the controller specs appear to be integrating with the views, but we have no guarantees about the full-stack happy paths through the application.

Bigger ideas:

6. Drop Ruby 1.8 support. Ruby 1.8 was end-of-life'd on 6/30/2013, and Travis currently has a build error on 1.8, anyways. This will also enable us to...

7. Upgrade to Rails 4.1. Rails 3.2 is no longer supported, so we should upgrade.

8. Isolate unit tests from Rails, and other units. We have to load and boot all of Rails in order to run any unit test. It'd be nice if we only loaded each unit's direct dependencies, giving us sub-second test response times while refactoring, and this will highlight more opportunities for refactoring, mostly related to cohesion and number of dependencies. Probably only feasible for the domain models; I haven't yet figured out how to effectively test a Rails controller in isolation, and am not optimistic about that changing. :(

So, all those who have commit access to Calagator, what do you think? If we were to start submitting Pull Requests along these lines, would you be amenable to them? Any other things you would like to see happen to the codebase? Other thoughts? Thank you for your time!

[origino...@gmail.com]

Hi folks, just an update here. We conducted our first refactoring session today, and it went well! We decided to just focus on the Event model for a while, because that's the biggest class, and probably could use the most attention. We improved a few small things, mostly related to making the code more idiomatic:

https://github.com/calagator/calagator/pull/74 - Remove all redundant `return`s from Event.

https://github.com/calagator/calagator/pull/75 - Remove all redundant `self.`s from Event. 

https://github.com/calagator/calagator/pull/76 - Fix Travis on Ruby 1.8 so that we're green again.

In summary, we had a great time, and are looking forward to doing it again! Anyone here interested in merging these, or providing feedback?

[Audrey Eschright]

Thanks for getting this rolling! I'm sure you and your students will find plenty of material to work with. Some parts of the code base have changed very little since early in the project. 

Reid and I will both be at Open Source Bridge this week if you want to talk through any of this in person. The hacker lounge is open to anyone who signs up for a free community pass. See http://osbridge.eventbrite.com

Audrey

--
You received this message because you are subscribed to the Google Groups "PDX Tech Calendar" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pdx-tech-calen...@googlegroups.com.
To post to this group, send email to pdx-tech...@googlegroups.com.
Visit this group at http://groups.google.com/group/pdx-tech-calendar.
For more options, visit https://groups.google.com/d/optout.

[Micah Geisel]

Great! I'll see if we can't arrange a field trip.