Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Cleaning bad code

gamasutra.com

Post reply on HN
Print view View original

11–20 of 22 posts

Re: Cleaning bad code

#11

Post by gbog »

I'm cleaning code right now and do it 90% in line with these good advices.

However, I differ on the over engineering issue. It is not enough to say YAGNI. With YAGNI people will put a value in a new column of table, then later on they will need another one, another column, then in fact we need to know when these value where set, by who, and you get a table with 600 columns, while what you needed to do from start was a simple event table, but YAGNI, you know.

So, no, you need to think carefully for some parts of your code about what the future might look like, specially for data structures and storage part. You need to avoid the many traps, and only with some careful consideration and some experience will you be able to do "the simplest thing that works" the correct way.

So what is the correct way? This can be easily seen in examples: to me vim, git, Linux, gnu coreutils, python, seem to be designed the correct way, even after a long life (but git) they still have many user, the base is solid and can support many fast changing projects.

Re: Cleaning bad code

#12

Post by gruseom »

16s wrote: post #8

One thing that I often do when dealing with old code that's performing low-level operations and outputting some binary format is to md5 the output. After that, I can change the code and I often do this when people ask for less memory usage, better over-all performance, etc. Once the code has been changed, I can prove that it uses less memory/CPU/whatever, and verify the md5sum of my output against the original output…

It's a good trick, but you can't md5 all possible outputs. How do you choose the inputs?

Re: Cleaning bad code

#13

Post by evilbit »

praptak wrote: post #10
einhverfr wrote: post #9

Most of this is really great advice. However I have run into horror stories that would shock you. For example: 1) Delete most of the comments 2) Stuff breaks. WTF? The code was processing its own comments! Also I would add a third option here (the option we chose with LedgerSMB) which is: "Localize the bad code, and replace with good code, a block at a time." Some code can never be cleaned up effectively. In this cas…

> 1) Delete most of the comments 2) Stuff breaks. WTF? The code was processing its own comments! My campfire story: change method name, stuff breaks. Method names had been parsed in a wrapper object (Python's __getattr__ being abused) so that calling 'setFoo()' triggered a 'Foo_changed' event. In fact, the method names constituted a mini-language with its own grammar and semantic. Apparently someone had gotten high o…

Corollary: AOP is the enemy of refactoring.

You unwittingly changed one of the (seemingly unused/unnecessary) pointcuts and your aspects aren't firing anymore. While not impossible to track down, these problems are difficult to resolve.

Re: Cleaning bad code

#14

Post by cpeterso »

My favorite technique for taming legacy code is to define facade interfaces [1] that carve out new modules. When I'm trying to decipher old code (or a big new library), the facade captures my understanding in a simpler API using terminology familiar to me. I can quarantine ugly code and "discover" modules that may have evolved out of the original code. Facades are also a great place to insert debug logging and assertions.

With a well-placed facade, I'm like Alexander cutting the Gordian Knot of code dependencies. :)

[1] https://en.wikipedia.org/wiki/Facade_pattern

Re: Cleaning bad code

#15

Post by hypnocode »

You could pretty easily title this "writing code right the first time" and some adjustments to tense, I don't think anyone would know the difference. Great article.

Re: Cleaning bad code

#16

Post by mjfisher »

gbog wrote: post #11

I'm cleaning code right now and do it 90% in line with these good advices. However, I differ on the over engineering issue. It is not enough to say YAGNI. With YAGNI people will put a value in a new column of table, then later on they will need another one, another column, then in fact we need to know when these value where set, by who, and you get a table with 600 columns, while what you needed to do from start was…

I don't think that's how YAGNI should be applied. I've always found it to be a great rule of thumb provided you're also constantly refactoring.

So in your example, perhaps the first column addition is justified. When it comes time to add the second column, that is the time to take a step back and think of a more general abstraction. This is because you've just proved for the first time that you actually need one. If you can't absolutely predict future requirements (and humans generally speaking are bad at this), perhaps you'd only ever have needed that one column in the first place. That way you keep a clean, simple codebase until you know a more complicated framework or pattern is required.

Of course, a rule of thumb is just that; I've seen YAGNI far too religiously applied too. And I'm in absolute agreement about being careful to think ahead carefully about the overall design and architecture of what you're working on - otherwise it's easy to become trapped in local minima of your solution space. It goes without saying different projects are suited to different approaches - YMMV.

Re: Cleaning bad code

#17

Post by davidcuddeback »

lifeisstillgood wrote: post #2

>> On the other hand, the long-term effects of never cleaning your code can be devastating. Entropy is the code-killer. I love that quote. Can I have "Entropy is the code-killer" on a T-Shirt please.

You can make one on CafePress. Here's one that I just made: http://www.cafepress.com/cp/customize/product2.aspx?from=Cus...

It's a little pricey for a t-shirt---probably because they only expect one sale. I don't know if there's a better way to design custom t-shirts.

Re: Cleaning bad code

#19

Post by praptak »

evilbit wrote: post #13
praptak wrote: post #10

Earlier quoted context omitted.

> 1) Delete most of the comments 2) Stuff breaks. WTF? The code was processing its own comments! My campfire story: change method name, stuff breaks. Method names had been parsed in a wrapper object (Python's __getattr__ being abused) so that calling 'setFoo()' triggered a 'Foo_changed' event. In fact, the method names constituted a mini-language with its own grammar and semantic. Apparently someone had gotten high o…

Corollary: AOP is the enemy of refactoring. You unwittingly changed one of the (seemingly unused/unnecessary) pointcuts and your aspects aren't firing anymore. While not impossible to track down, these problems are difficult to resolve.

I am not sure if this was AOP though maybe I'm committing the no true Scottsman fallacy here. Side note: decorators were not present in Python then, so there was no easy way to do the cutpoint thing in a visible way.

Re: Cleaning bad code

#20

Post by ldh »

evilbit wrote: post #13
praptak wrote: post #10

Earlier quoted context omitted.

> 1) Delete most of the comments 2) Stuff breaks. WTF? The code was processing its own comments! My campfire story: change method name, stuff breaks. Method names had been parsed in a wrapper object (Python's __getattr__ being abused) so that calling 'setFoo()' triggered a 'Foo_changed' event. In fact, the method names constituted a mini-language with its own grammar and semantic. Apparently someone had gotten high o…

Corollary: AOP is the enemy of refactoring. You unwittingly changed one of the (seemingly unused/unnecessary) pointcuts and your aspects aren't firing anymore. While not impossible to track down, these problems are difficult to resolve.

This worries me every time I use AOP, but I mitigate that risk by writing integration tests which assert that the aspects are doing what they are intended to, and also to raise an alarm if somebody unknowningly breaks them.
Post reply on HN