Related link: http://c2.com/cgi/wiki?DontRepeatYourself
Question for programmers:
How do you know when you’ve found the right solution?
This weekend I was working on one of the toughest but most satisfying projects I’ve ever done. Everything was coming together beautifully. Things that had been messy problems for 8 months were all getting neatly solved using best practices (especially D.R.Y.) - I was really on a roll.
… and then I got stuck on a function that looked like this:
function blah($id, $rare_option='n', $seldom_used='', $something_else='', $ugly='') {
switch($id) {
case 'a':
return $id . 'y__p' . $rare_option;
break;
case 'b':
return $seldom_used . 'x|x|x' . $seldom_used;
break;
case 'c':
return 'CDB_12345' . $something_else . $rare_option;
break;
case 'd':
return $ugly * $ugly - $seldom_used;
break;
}
}
(The above example is fake. The real function needed to create output filenames based on partner-company preferences, of which there are 5 or 6 different formats, based on different information inside the file.) I spent almost an hour trying to simplify it, but unfortunately that was as far as I could get. It was a drag to leave something ugly like that, knowing I’m going to have to look up the function every time I use it in the future, to find out what list of variables I’m suppsed to pass it, and in what order.
Which got me wondering how other programmers handle this situation: when you’ve written something that WORKS, but goes against your good taste in design. Is the ugly solution ever the best solution? Is it not right until it’s as simple as possible? I keep thinking that ugly solutions are always a sign of the wrong approach. Is anything that works "good enough"? Or do you strive to find the truly beautiful solution to everything? Thoughts, anyone?
Thoughts, anyone?
Beauty is relative
Elegance is determined by how well the solution fits the problem, not whether it looks ugly out of context.
elegance is overrated
I posted a response on my blog with trackback here, but I don't see the trackback. Maybe a bug somewhere. Anyway, my short answer is that when you're a young programmer, elegance is a very attractive guidepost, but with many years of experience, other things become significantly more important. Readability and encapsulation seem to be at the top of my list. Anyway, the blog piece says more.
Document, Document, Docu....
You get the idea. Constraints on functionality, time, etc. often prevent the very best (read: elegant, efficient, whatever your criteria) solution from ever being included in your code. That's why you document the heck out of your more difficult stuff, and just remember to go back to that documentation.
For PHP, I've found phpDocumentor (www.phpdoc.org) is a great tool. I add code blocks to code like what was listed in the post, and have phpdoc do its thing. When I need to remember what params a function takes, or "Did I write a function to do X?", I take a look a the auto-generated documentation. Very helpful.
Vocabulary
Your first instinct is right -- this is ugly.
Rather than bang your head against the wall, post a link to the actual code in question and someone here will fix it for you :-)
It's not that you can't fix it yourself, it's just that you haven't already solved this problem so the pattern is not obviously apparent to you, and you can't budget the time to exhaustively work through it.
One way of learning is from the experience of others. Post the code. Expand your knowledge for next time.
Vocabulary
Oh yeah, I think time is better spent coming up with the right solution rather than documenting a poor one.
the actual code
> post a link to the actual code in question
Nah - no point. I'd have to sit and explain the entire massive system. I think I've got some ideas on alternate approaches. I was more concerned about the general approach and philosophy than this specific function.
Vocabulary
In an ideal world, I would agree with you 100% regarding spending time on the 'right' solution instead of documenting the 'wrong' one. In the real world however, there are several factors that may exist. Time contstaints may dictate that this HAS TO go out the door today. Yes, sometimes 'HAS TO today' can be an overstatement, but business conditions may force you to ship a less-than-ideal product in order to make sure you reach your market on time - think Microsoft (semi-kidding there). Time may also dictate (although probably not in this simple example) that the 'best' solution isn't always used. For example, I'm sure you've used an API call to read a file some time when you could write your own low-level code that would be a 'better' solution in terms of efficiency. Would it make much sense in terms of development time? Probably not.
I guess what I'm trying to say with all that blabber is that unique situations really dicate unique reactions. For something like this, it's actually very difficult to apply a hard-and-fast rule that could encompass every situation you would ever encounter.
the best solution is the one that people will understand
You can write a beautiful piece of code that brings tears to your eyes from joy seeing it, but if the guy coming after you has to spend several days trying to figure out what it does because you made it too terse, too abstract, to be easily understoof it's not production grade code.
The code you show does the job, it's (relatively) easy to enhance and modify.
Therefore it has a beauty all of its own and is a good solution.
It may not fit your ideals of esthetics, but esthetics play very little part in professional coding.
Ease of maintenance and delivery in time and on budget are all that matter.
the best solution is the one that people will understand
Very good points! Thanks jwenting.
Yes it's ugly...
And it's definately not the best solution.
You want to refactor that type code ($id in your code) using the following pattern from Martin Fowler's refactoring book:
Replace Type Code with Subclasses
I recommend the book, it is the encapsulation of many years of the communities experience in object orientation and will let you spot common problems.
And if you think object orientation is abstract and namby-pamby (and since you already mentioned DRY) check out the pragmatic programmers "OO in one sentence" article (PDF):
Keep it DRY, shy, and tell the other guy
the best solution is the one that people will understand
I think aesthetics are *very* important in programming, they subliminally let you know when something is wrong e.g. in this code adding each new format involves tinkering with working code.
However I agree that too clever code should be avoided at all costs and you should Kill Your Darlings whenever you find yourself overly impressed with your own coding skill.
Someone once said that it's twice as hard to debug code as to write it, so if you are writing code that stretches your talents to the maximum then, forgetting about co-workers for a moment, even you are not smart enough to debug it.
Yes it's ugly...
I just noticed this, from your best practices link:
Heuristic #5.13
Explicit case analysis on the value of an attribute is often an error. The class should be decomposed into an inheritance hierarchy where each value of the attribute is transformed into a derived class.