bcholmes: (Default)
[personal profile] bcholmes

It took me a much longer time than I'm proud of to see a difference between bad programming practice and different stylistic convention. Like the whole where-do-you-put-the-curly-brackets argument (when I programmed in C++, I put them on lines by themselves -- it made it easier to handle #IF/#IFDEF preprocessor instructions, and I still think it's easier to visually parse the code structure. But now that I program in Java, I follow the Sun conventions).

Every once in a while, I see something and try to figure out if it's bad programming or just different.

Thought Number 1:

There's a programmer that I've worked with at my current job. I have huge respect for his programming skills, but often had difficulty with his unwillingness to change his behaviour because of any process considerations.

One more than one occassion, I saw him implement a something like a cache. His implementations would always extend java.lang.Thread. The run() method would then periodically prune old items from the cache.

Cache Class Diagram 1

I always thought that this hugely strange. Me, I'd create a cache that holds on to a CacheCleaner class. I'd make the relationship compositional, and I might even make CacheCleaner an inner class because it's so specialized.

Cache Class Diagram 2

My cow-orker just didn't see that there was any difference. Now, a lot of people, I'd just tend to write off their implementation as bad, but I had a lot of respect for this guy. Nonetheless, it feels unnatural to me to say that a cache is a thread. I don't get it.

I think that part of my thinking on this has been influenced by an article I once read in Java Report that categorized methods into types -- getters/setters, command methods, initializers, factory methods, etc. In my mind, a thread object should really only have thread-related command methods; run() being the primary one.

(And, as an aside, I'm often amazed at the number of Object-Oriented developers who don't understand how the compositional relationship is different than a simple association).

Thought Number 2:

The Eclipse tool that I use for almost all Java development that I do has a number of cool features that help you spot unnecessary code. I can turn on a feature that marks all unused local variables, for example. It's cool for spotting a wide variety of mistakes and code bloat.

One of the features marks unnecessary else statements. Consider the following example:


if (firstName == null || lastName == null) {
    throw new InvalidNameException("The name is foo-foo");
} else {
    name = firstName + " " + lastName;
}

The tool would then tell me that the else was unnecessary and would suggest that I remove it. Me, I don't like that suggestion. I guess I always think about method implementations in the graph theory sense (yeah, I know too much about McCabe's Cyclomatic Complexity and crap like that). My code snippet has two code paths (one that ends with exception and one that does not. I like the way the if-else visual structure translates (in my head) to a code graph.

In my mind, to take out that else would be to treat the exception as a GOTO (which, yeah, in some senses it is).

But I've worked with a number of programmers who seem to be allergic to too many levels of indentation. They'll do anything to avoid a level of indentation. Me, I'll do anything to avoid something that smells like a GOTO.

Assertions and else statements

Date: 2004-12-04 09:21 pm (UTC)
From: [identity profile] xthlcm.livejournal.com
Re: Thought Number 2, a few years ago I converted from your line of thinking (the if...else pattern) to discarding my else statements. I actually think it makes for more readable code. This happened because I started using asserts to deal with constraints when entering methods, and then that pattern tended to infect any non-exception-generating-but-abortive-conditions further down the buffer. My thought process when coding something like this is more like:

if ( [there's an exceptional condition] )
{
     [return or throw] [something appropriate]
}

[Continue doing stuff]


Particularly in lengthy code blocks, I think this makes things much easier to scan through methods.

(no subject)

Date: 2004-12-04 10:44 pm (UTC)
From: [identity profile] jodawi.livejournal.com
I think exceptions are GOTO, and they're nigh unto evil in C++ GUI apps unless handled very carefully. Because they're "GOTO someplace somewhere that may or may not exist, which you may not know about until your users report crashes due to unhandled exceptions", rather than going to an easy-to-find location. They also seem to dramatically bloat code size, according to something I saw long ago. I have vague memories that Java handles it all better.

I get rid of the else. Particularly if there's several tests in a row, and I'd end up with lots of nested brackets. The exception is an abnormal situation which I don't expect to happen, and is not really part of the purpose of the function which deals with what I do expect.

Thought #2

Date: 2004-12-05 04:28 am (UTC)
From: [identity profile] hellsop.livejournal.com
The ELSE is *much* easier to maintain in six years, after several hundred lines of code have been added to the first branch and it's been entirely forgotten that the next step is an unconditional one. At some point down the road, someone's going to decide that the processing should NOT flag exception condition and should instead continue processing after handling the condition. EG, the program now has 60 lines of code that query Google for celebrity status, and if sufficient hits, it reassigns some cases of lastname == null to move firstname to lastname and null firstname. At that point, the unconditional happens, and you end up with the wrong result: "Cher". Working on what's now MY OWN CODE six years on, I've been more than glad several times that I've indented even a dozen iterations if necessary to FIND all these things, and being careful to never mainline a conditional, and never leave a conditional that "is always true".

#1

Date: 2004-12-20 11:17 pm (UTC)
From: (Anonymous)
Why having an invalidation thread in the first place? This adds to the complexity of the code. Why not checking for invalid entries when putting things in / getting things out. Other than that, I really think this is a matter of taste and not a technical problem.

Oliver

Profile

bcholmes: (Default)
BC Holmes

February 2025

S M T W T F S
      1
2345678
9101112131415
16171819202122
2324252627 28 

Most Popular Tags

Style Credit

Expand Cut Tags

No cut tags
Powered by Dreamwidth Studios