3 ms·
Yes, but when the guy pointing ugly code is a good coder, "ugly" is just a shortcut for something that usually can be explicitly stated and is really harmful. (
by gbog 15y ago
Yes, but when the guy pointing ugly code is a good coder, "ugly" is just a shortcut for something that usually can be explicitly stated and is really harmful. (A bad coder will say "ugly" for any kind of code he did not write.) Here are some of the reasons behind "ugliness":
- Wrong logic giving correct results (this is awful)
- Hidden dependencies making remote changes dangerous (bèè)
- Misleading variable or function name
- Unnecessary use of magic (eg programmatically building function names), this breaks a lot of code analyser, and makes your code base ungreppable, which is very bad.
- Code duplication and configuration factorization.
- Nasty hack breaking the model (eg changing request parameters on the fly)
All in all, I agree with the OP premices, but not with his solution. Adding comments all over the code is not going to save you, and making sure you understand all corners of the module you work on before starting to modify a comma is not that helpful either. In front of a montain of legacy code that need some ordering, if it is a short cycles mode with versionning, I would:
- First make sure my tools are accurate and fast. It mostly means tweaking my grep alias, so I grep all the code and nothing more. I may mean to find a way to run the modified code in less that 3 secs (unit tests can help for that part).
- Second, don't try to understand all the thing, start low, maybe rename a few variables that are obviously wrong, for example marking them as private (adding _ prefix in python).
- You won't know were are the painful points in the begining, so you should continue doing safe & simple refactorings, one step at a time, in order to slowly "wake up" the code. A bit of temporay commenting can help, like "this func returns a x-blob used by that module".
- Then when you become familiar with the code you'll be in position to decide if and where to do some real changes. Some people here seem to believe that legacy code should not be changed except in case of earthquake. I don't agree. Adding a pile of hacky crap over a pile of hacky crap to make the thing temporary work is not a good bet on the future: do it only if you plan to leave your company soon enough.
- Doing a bigger change might require some iterations and lacemaker work. If possible, you can bridge the legacy code, ie write a func_new and compare it's result with func_old during one iteration, to kinda prove that they are the same.
- I noticed that it is sometime useful to take a big class, a big god-like object doing too much, and cut it in much smaller pieces. Sometime you may have to re-glue together some pieces because you went too far, sometime you may even kind-of rollback your changes, but in any case these operations will shaken the code base a bit, which is good.
These operations require courage regarding of the code base, because you will feel lost sometime, you will fear regressions, you may even fix a functionality that should not be working and, consequentially, break something. Here, be careful, you should not be fixing broken parts and refactor in the same operations, except if you know enough to be sure. One purpose of refactoring is to allow you to fix broken (or slow) part later. I insist on "later".
It is also requiring patience and tenacity in front of the operational team: they will think you are doing nothing (they are right, from their point of view, per the very definition of refactoring).
- joelthelion 15y agoThis is exactly why these words should be banned. What you fail to realize is that "good" or "bad", when applied to all these things, depends a lot on the context. You don't expect the same level of quality from a one-page script that will be used once, than from a crucial part of a large-scale program involving thousands of people. Spending two days making the script robust, modular, flexible and unit-tested is just as bad as pasting an undocumented hack in the large program. We need to try to be rational and stop mixing morals and code.
- gbog 15y ago> We need to try to be rational and stop mixing morals and code. It's funny because I think you are wrong. I would agree with stopping mixing aesthetics and code. The highest poetry from, say Mallarmé, has qualities that would be considered as stupid plain wrong pure evilness in code. But morals and code have relations. A lot of relations actually. My moral forbids me to steal and cook my neighbour's cat, not because it is forbidden, but because I have enough self-respect to not behave like that. The same applies when writting code. Even if operational pressure is high, I am not the kind of guy who will let the shitty work to other coders behind me. I have self-esteem, and don't want peer coder to think I am the worst shit in the world, so I just won't add crap to a pile of crap without doing reasonable efforts to do at least some cleaning. Like in the toilets: let it as clean as would have liked to find it yourself. Maybe we should first agree on what is "morals". For me it is not a set of predefined values like "good / bad", it is the very reason why we behave properly to each other (or not), it is a consequence of self-esteem. Oh, and I think a one-shot script do not need to be refactored to be "beautiful" mind you, no, I think it should be either trown away right after use, or at the very last marked very heavily in the file name and as the first uppercased line: one shot script, no fixing required.