Live data from Hacker News

How A Pull Request Rocked My World

clayallsopp.com

1–10 of 158 posts

Re: How A Pull Request Rocked My World

#2
IIRC, Fabio works at the Swiss Ruby shop Simplificator, who are all very nice chaps indeed! Glad he helped you along.

EDIT: I just checked, yes he does (well, unless we're talking about another Fabio of course, which is possible...)

Re: How A Pull Request Rocked My World

#4
I don't love this refactor. I especially don't like how it generates names from other names -- e.g., given 'switch'/'submit', appends '_row', and then converts it to CamelCase so you get 'SwitchRow'/'SubmitRow', then instantiates the class named that.

Here's why: When I am maintaining somebody else's code, even if I guess that this is going on, I rarely trust these kinds of name-generation tricks. When I need to change some code, I'm going to grep the codebase for 'SwitchRow' and add a new line in whatever conditional to instantiate my new class. When grep can't find any instances of SwitchRow's constructor being called, I become suspicious and it costs me more time, because I am making sure I properly understand what is actually going on.

Re: How A Pull Request Rocked My World

#5
post #3

The classic "Replace Conditional with Polymorphism" design pattern, which took me way to long to realize that it also applies to Python.

Indeed it does. I have a large code base at work that uses this pattern for Django models with a bit of metaprogramming to create a template for types of input sensors. It's amazing what this kind of refactor can do when you need to turn a long conditional into something more generally extensible. New sensor types with custom column configurations take an average of 5 lines of code now, instead of about 30 before.

Re: How A Pull Request Rocked My World

#8
I don't agree with this change. Based on the code, it looks like there should be an enumeration of cell_types somewhere, a button should have a cell_type, and then there's a map from cell_type to construction_fn (or a polymorphic set of classes to serve as the map.)

Metaprogramming is something you do when you can't do it easily in the usual way. If a dispatch table or inheritance takes care of something, use the existing language features instead of building your own.

(EDIT: On examination, there already is a RowType, and the pull request is a lot closer to my suggestion than I first thought, so mostly ignore this.)

Re: How A Pull Request Rocked My World

#9
Refactors like this are super cool and often needed but I sometimes have to think why we don't store the easier to read code as well.

I know we have git and we can always do a diff to see what changed. What I am talking about is a more holistic view. Being able to search a code base for SwitchRow and then somehow the code comes up where it was refactored into this dynamic programming style.

Git can do this but only if you are actively looking for it. I wish you could do a metacomment on the refactor without cluttering up the code.

Post reply on HN