Hacker News new | past | comments | ask | show | jobs | submit login
Refactoring to clean code. (amaxus.com)
52 points by handybitesize on Oct 5, 2010 | hide | past | favorite | 13 comments



These are excellent examples of refactoring.

It's strange, however, that the author brings up the absolute necessity of unit tests when refactoring like this, yet says he omitted them for brevity. I've found, however that one of the most difficult and important parts of refactoring is making sure code has proper test coverage, especially when starting with crufty code. It's unlikely that the (not very clean) code he presents came with a thorough set of unit tests.

The first thing I try to do when working with insufficiently tested code is to begin refactoring it into a more testable form. This involves many small steps, and in some cases manual testing to make sure that the automated tests I'm writing are testing the correct thing. I think this article would be better if he moved the parts conducive to that (#6 and #9) to the top and included examples of writing tests (with appropriate mocks).

Further, canvas is a rich and complicated API that is relatively difficult to test against; for this article something simpler might have been more appropriate.


This is a phenomenal introduction to refactoring to clean code. A lot of young developers with whom I interact tend to not understand the importance of cleanly refactored code.

The hacker mentality, furthered perhaps by the prevalence of scripting languages, allows people to think sometimes that "if it passes the test, it's good for production!"

This is how amateur software projects die. I've seen it dozens of times: a young, ambitious programmer starts promising project, creates a sort of mock-up that runs (badly), and then tries to continue adding features and improving upon the already-decaying codebase.

Refactor your code. Make things clean. Even if you never plan on passing your passion project on to another developer, it's important to understand that without clean code even you will be unable to understand and efficiently work with your codebase.

This is one of the better introductory tutorials I've ever read, and the hands-on example is excellent. Great read, and I've bookmarked it to refer people to later.


It might be because the example is somewhat contrived, but am I the only one who sometimes finds refactored code harder to understand or change? The final version is "cleaner", but it also is twice as long, which means you have more code you need to understand, and more text to sift through to find what you need.

I often find the "many little functions" working against me. An abstraction is beneficial if your task doesn't require seeing through that abstraction - but otherwise it can be harmful. If I need to change something core to movement, flap() and drop() aren't doing me any favors.

Unless theres other glaring issues, if a function is already less than 30 lines of code, I usually find a comment nicer than refactoring to yet another method.


How can code be both "clean" and "twice as long"? That makes no sense to me. If one can make a program half as long (I mean logically, not lexically, such as by shortening names) then it obviously has problems. Since bugs grow approximately linearly to code size, this is a big deal.

As for "many little functions", although this style is favored by the OO elite, the research literature is actually against it. Moderately sized functions (IIRC, up to 100 lines - but the studies were in old languages) have been found to be less error-prone. Steve McConnell and Robert Glass have both written about this. I've posted citations here before but don't want to right now.


> am I the only one who sometimes finds refactored code harder to understand or change?

No, you aren't the only one. I made a similar case in a related discussion a few months ago:

http://news.ycombinator.com/item?id=1224210 http://news.ycombinator.com/item?id=1224206

This particular tutorial seems to have some merit, but it's a bit too clever for its own good at times. For example, the very first change introduces a named constant called SIGHT_SHOT_RADIUS_PX, which is not in fact used as a radius at all. Indeed, if it were, the later refactoring to split out a testForHits operation that in turn uses testForXAxisHit and testForYAxisHit would break down, because while testing for hits is a useful atomic operation, the tests on the two axes would not be independent and the logic in those functions is meaningless out of context.

No unit test is going to warn you about traps like these, and breaking things down according to dogmatic rules rather than to separate meaningful, self-contained concepts does tend to create such problems IME.


I prefer explanatory variable and method names to comments. Comments lie: they cannot be specced or debugged, and few if any developers take responsibility to edit and maintain comments made by others.

As a rule of thumb, I comment only when there is a surprising externality relevant to the code e.g. performance, obscure workaround.


In my observation, many programmers who advocate liberal commenting have no discipline whatever when it comes to maintaining all those comments. That ought to disqualify them from any debate on the subject. Unfortunately, except for the few one has personally worked with, it's impossible to tell who they are. :)


Refactoring is merely changing code without changing what it does. If you took a bunch of methods and combined them into one, that would also be refactoring.

Now, you may disagree with people on what constitutes "better code", but that's not a knock against the practice of refactoring.


I have a love-hate relationship with unit tests. They are great in theory but in practice not enough people do it to reach that "critical-mass". By that I mean very few projects you pick up (especially in the maintenance phase) will have tests.

And writing good tests is HARD. If you think it is easy then you are testing the wrong thing. A lot of people say refactoring without tests is pure danger. I disagree. Sometimes you don't know enough about the problem space to write good tests. However, refactoring forces you to understand the "micro" world of the code. And this gives you a leg up in order to understand what tests are actually needed.


Do you have experience with QuickCheck? It allows you to specify rules and the computer will generate the tests for you. That's very pleasant to work with in languages susceptible to this kind of reasoning (e.g. Haskell).

(Edit: quicktest -> QuickCheck. Thanks!)


I suggest that you're thinking of http://en.wikipedia.org/wiki/Quickcheck


I've been working on a project for the last year or so (on and off) refactoring code and shifting it from Classic ASP to C#/.NET.

I'll say the one thing that would really help would be unit tests on the old code. It's rough transitioning some dirty logic that seems to work from one end to the other and not having a simple test to know I didn't break everything upon refactoring.


I have the book "Working Effectively with Legacy Code"[0], and it's pretty much just "Put things under test, then change them.". Still a useful read, though, if you find yourself working in that sort of thing often (I do).

[0]: http://www.amazon.com/Working-Effectively-Legacy-Michael-Fea...




Consider applying for YC's Spring batch! Applications are open till Feb 11.

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: