There are many kinds of ugly
gamedev.net
gamedev.net
If you want to criticize code, say exactly what's wrong with it, so we can have a detailed and rational argument about it.
If you say "this script is ugly, it should be rewritten", there's not much discussion that can be had about it. If, on the other hand, you say, "this script is written in a way that is not modular enough", then we can discuss whether modularity really is a requirement or not, for that specific script in that specific project.
It's become an almost meaningless word when people use it to describe a snippet of code that scratches some specific itch that bothered them as opposed to the overall quality of the code.
- Decoupled - Light - Explicit - Straightforward - Humble - Balanced - Self-explanatory - Obvious - Neutral - Delineated - Malleable - Greppable - Luminous - Harmonious - Limited - Dense - Short - Homogeneic - Unique - Complete
Some qualities of bad code:
- Magic - Intrusive - Terse - Long - Convoluted - Implicit - Bizarre - Complex - Personal - Creative - General - Colored - Variable - Grandiloquent - Poetic - Cryptic
(Just my personal list of synonyms for good or bad code)
- 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).
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.
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.
Sometimes "ugly" really seems to hit the nail on the head, like when code fails to communicate what it does or why it works (e.g., due to an implementation that looks like it resulted from brownian-motion trial and error).
For me, the thing which halted that ivory tower mindset was changing languages. I find that after spending a lot of time in a single language, I start thinking along that language's lines when I'm designing software. There are a lot of things which are a good idea in some languages and rubbish in others. Eg, enums are great in C, but useless in Javascript. Interfaces and abstract classes are silly in ruby.
I think there's a trap programmers fall into with this stuff, where we get used to some method / philosophy. After awhile, its easy to forget that the method comes with a price that is being paid daily. Only by periodically re-evaluating all the little decisions you make can you start to get a real sense for when those decisions are appropriate.
But as for the code rot problem, there are two solutions I've seen which made a difference:
- Test-driven development (full on, no cheating, tests before you write code. 95%+ code coverage) - Code reviews before every commit
But both of these methods will slow you down heaps in the short term.
I find proper TDD makes me about half as productive in the short term. In the long term, it slows down big redesign work because there's twice as much code to change. The benefit is that you can formalise all your obscure edge cases in tests. If your test coverage is good enough, you can safely refactor code without breaking anything. This removes fear from development and lets you make clear decisions about design. (Also, its amazing how many bugs you find when you start writing tests. It is humbling.)
I suspect that most of the benefits of code review comes from simply setting aside time to talk about code style. It consumes a lot of time. In exchange, you get higher quality code.
How much you care about speed and code quality really depends on what the project is. You need to try them both out some time to be able to make that decision properly.
IMO that is miles from the truth. I handle code that "just gets things done" everyday and it is a real pain. Elegant code is not code that is just well formatted and clean, is code that solves a problem in an elegant way, a way that will scale better and that is easy to understand and maintain.
The real problem is that it's really really hard to get managers and clients to understand to value of elegant code.
When hell that is left after code-like-hell to get things done gets in the way of maintaining the code, the blame gets thrown in the programmer that didn't write good code. But they never want to trade off time for quality. And when we ask for time to get things done with quality, they assume it's an ego thing...
If you have half as many lines of code, -- it takes half as long to read it -- it may be twice as easy to explain -- you may be able to fit it all into your brain at once -- it compiles faster -- it may run faster
Remember the slowest processor on the planet is your wetware. That's the place where space and speed are critical factors.
Is the code badly formatted? Poor choice of variable names? Non-idiomatic language use? Monolithic code blocks? Spaghetti code? Depreciated methods? Dangerous hacks?
Ugly could mean any of these & more. Some are less tolerable than others.
Admittedly, there's room for interpretation and a degree of subjectivity here. However, this doesn't prevent us from knowing about what dirty code is and I don't see a compelling reason why we ought to tolerate it (e.g. long functions with a lot of branching are difficult for everyone to read, so you shouldn't have any of those; same goes for badly named variables/functions/classes).