Hacker News Reader

Comment by lincolnq | Hacker News Reader

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.

View on HN

...I rarely trust these kinds of name-generation tricks.

So you're probably not a big fan of Rails, then?

and what does `trust` mean? You don't trust that correct object won't be given back to you?

I suppose more like trust whether the resulting code will always be correct.

what about using a multiline YAML text to be loaded with automatic generation of objects from class name strings? (reminder!)

I'm actually not a huge fan of implicit name-generation tricks, but I'm also not entirely opposed to them.

The trick here I think is consistency - is name-generated type dispatch the standard across the entirety of the codebase? If it is, go wild. If it's not then I'd be much more wary of it.

Consistency and building correct expectations for future maintainers is pretty important. Doing a smart trick in one place but failing to do it in other places where it makes sense IMO tends to create a bigger mess than doing it the dumb-but-safe way everywhere.

I so agree with you. There are few programming mistakes that consistency in use does ameliorate. On the other hand, no matter how "good" the code, if it's inconsistent, it's bad.

I disagree. Consistency is only valuable so long as it helps clarity and understanding. While that is most often the case, there are exceptions.

Agree. Names should not matter. You are going to have to document name construction. If your library users use IDEs you are going to have to get them to understand names.

This is especially true for those of us who do application support.

When we are investigating application stack traces from production for a large application, it is awful when you discover that the developers decided to be clever with dynamic function names, making a large code base very difficult to support based on available logs.

If you think you're being clever you're probably just being a dick.

Given this is ruby code, I think it's acceptable, and well-structured enough to be understandable.

While the implementation itself might be a bit 'eugh' because of the meta-programming involved, there was enough thought put into code organisation to allow you to grep `RowType` to find the module that namespaces all these dynamically instantiated classes.

Hell, if the code in the tree was arranged correctly, then `tree | grep row_type` would give you the folder where they all lived.

Agreed. This "refactor" is actually a hack. It shouldn't pass code review.

Reply on news.ycombinator.com