Don't "Namespace" Filenames

When I first started this blog, I wrote up a rant about directory names that mislead about what their contents are. At the same time, I complained about adding unnecessary morphemes to directory names, things like "db" or "model" or such-like.

I've recently become annoyed with the habit of -- for want of a better term -- "namespacing" directory names and file names. I'm not saying not to use good namespace discipline in source code. What I mean is having a directory structure like this:

  zoo/
    zoo_iface/
      zoo_cmds.hpp
      zoo_cmds.cpp
      ...
    zoo_model/
      zoo_cage.hpp
      zoo_animal.hpp
      ...

Why does everything have to start with "zoo"? Look at it this way: if that was how we named people, then everyone's last name would be part of their first name and also part of their middle name. Instead of calling someone Mary Jane Smith, we would know her as MarySmith JaneSmith Smith. Some people make it even worse by prepending the full directory name onto the file name -- like "zoo_model_cage.hpp". Poor Mary Jane would be MaryJaneSmith JaneSmith Smith.

Of course, a source file name may be dictated by its contents -- you're following our C++ standard, right? -- so my ire is directed at redundant directory names more than filenames.

This principle doesn't have to stop at source files.  It boggles my mind that raw bug testcases where I work start their life with names like this:

  /home/bugs/prod1_bugs/comp2_bugs/bug6003.tgz

For crying out loud, I know this is a directory full of bugs, can't we just call each bug something like:

  /home/bugs/prod1/comp2/6003.tgz

?

Filename Expansion in Tcl

Stupid, stupid, Tcl. If you want to run a shell command with a glob filename, like *.h, you can't just do:

exec ls -l *.h

You have to explicitly tell Tcl to expand the glob, because it will pass *.h to the shell literally. But you also can't do:

exec ls -l [glob *.h]

because it will pass the entire list to the shell literally (thanks, Tcl, that's useful). You get lucky and it works if there is exactly one file matched by the glob. Otherwise ls will complain that there's no file called "a.h b.h c.h".

So what do you do? Unfortunately, if you pull up the man page for exec on the internet, it tells you a way to do this that is only syntactically valid for Tcl 8.5 or greater. For those of us living in the past, the demonic incantation you must utter is:

eval [list exec ls -l] [glob *.h]

Don't use Tcl unless you have to.

Let's Allocate Stuff!

Nobody's perfect, of course, but when there are so many things wrong with a small piece of C++ code, it's hard to be placid. How about:

  std::vector v = new std::vector;
  // ...
    v->push_back(new std::string("..."));
  // ...
  std::ostream *str = get_me_a_stream(...);
  for (int i = 0; i < v->size(); i++) {
    *str << (*v)[i]->c_str();
  }
  delete str;
  // loop to delete v and its elements.

Ow, my head is spinning from all of these needless allocations. Let's not wear out new and delete.

Well, if you first learn C and then learn C++, your head might still be in Pointer Land. But the unnecessary call to c_str() makes it hard for me to get any work done.

Blessed Mother of Commented-out Code

It's the little things in life that give pleasure, isn't it? Like the lovely comment below, which has adorned a source file at my company for nearly sixteen (16) years now (meaning the file is at least 5 years older than the company).

/*****************************#if (0) //[
// Marked code has been deleted   -- XXXX (18 August, 1994)
// No need to check whether the value is X/X/X
// Only the size is to be checked which has been done
[[[ DELETED

      char  val;

      val=tolower(*((char*) var)->getValue());
      if ((val != 'X') && (val != 'X') && (val != 'X'))
          return 0;
]]]
   #endif //]
*************************************/

There are many facets to this beautiful gem. First you'll notice that there is a nice C++-style // comment -- complete with the date and the signature of Mr. XXXX -- itself commented out by a C-style /* comment. Too bad the dead code isn't commented with //, so that my grep for tolower would have stood out as unnecessary.

But look, really the /* is there to comment out an #if 0. The guy must have wanted emacs to colorize the dead code as a comment -- I don't think emacs had font-lock back then, but there was the hilit19 package.

Still, you can't be too careful, so let's also wrap that code up in a [[[DELETED ... ]]], in case there is some human that can't read the C comments and the if-0, but who will understand what "triple-bracket DELETED" means.

Finally, it appears the conditional was originally written as #if (0) [ ... ] -- was that ever legal C? -- but when that wouldn't compile, the brackets had to be commented out, because every character in this file is too precious to ever delete. This must be the bad code afterlife. You can check out anytime you like, but you can never leave.

How did all this happen? Is it an accumulation of different commenting-out strategies that occurred over time? Or is it just an unfortunate snapshot of one man's frenzied efforts to remove 3 lines of code without actually deleting anything? Sadly, we can only speculate on what happened, because these lines entered our repository in exactly this condition over 10 years ago.

I love deleting dead code, but I can't touch this one. It's an antique. And so ugly that it's beautiful.

Csh Skips Last Line in Script

Oh, joy.  I've found another thing to hate about csh.  If the last line of your script doesn't have a newline after it, that line won't be executed.  It will be silently ignored.  That isn't very useful.

If emacs knows you are editing a shell script, it will automatically put a newline at the end if you didn't do it yourself.  But if you have an ordinary text file that you will run with source, it better have a newline at the end.  You can get emacs to either ensure that or warn you about it by adding one of the following lines to your .emacs file:

;; Quietly add a newline if missing.
  (setq require-final-newline t)

  ;; Ask if you want a newline at end of file.
  (setq require-final-newline 'ask)

Googling for this problem, I found another nice csh rant.

Tcl Case Analysis Lunacy

This goes way back to the roots of this blog. Complaining about Tcl, and complaining about code that breaks down into unnecessary and confusing case analysis -- those topics were the first three posts here.

Unsurprisingly, the toy language that is Tcl is serving me up some ridiculous case analysis. It has to do with the C++-side of a Tcl integration. When you look at the internals of the language objects created by the Tcl interpreter, some things that are conceptually the same have different internal representations. It's bad enough that I have to succumb to case analysis to figure out what's what: but on the other side of that wall, Tcl had to do some case analysis to put them all in different representations! Bad, naughty Tcl.

Here's what's bugging me. When you look at a Tcl_Obj over in the C world, it has a typePtr member to distinguish different types of things:

• A simple name like A has a NULL typePtr.
• So does a list of things in braces, like { A B }.
• A name with some special characters like A[0] has type "string".
• So does a list of things in braces which extends over a few lines, like:
  

  { A \
        B }
  

• An explicit list like [list A] has type "list".
• A quoted string like "A B" has NULL. At least I think so. I lost track.
  

So obviously there are a bunch of if statements in the back-end tangling stuff up like this. But it's nearly impossible to untangle it on the C side.

Stupid Tcl. Stupid case analysis.

Don't Give Variables Negative Names

Sometimes programmers get caught up in the absence of something. Don't let that leak over into the name of a variable or data member:

bool Job::acceptable(const JobCandidate &applicant) const
{
  bool noFelonyConvictions = findFelonyConvictions(applicant);
  return noFelonyConvictions && qualified(applicant);
}

The noFelonyConvictions variable documents the code in a readable way, but it gets silly if the code ever changes so that you have to initialize it, or if someone ever wants to know if there are felony convictions. Things become less readable:

  bool noFelonyConvictions = true;
  if (applicant.findRecords(FELONY)) {
    noFelonyConvictions = false;
  }
  return applicant.qualified(!noFelonyConvictions);

It all makes more sense if you name the variable felonyConvictions, which initializes naturally enough to false, can be set to true if one is found, and can be negated to show absence. Also, if you later become interested in the number of convictions, there will be fewer code changes if the variable changes from a bool to an int.

Whenever you notice yourself putting a "no" into a variable name, get rid of it.

Nice Code Structure

Here's a brilliant one I just ran across at work (variables changed to protect the... to protect someone):

  for (i = 0; i < lim; i++) {
    if (someCondition(n[i])) {
      warn("you can't do that");
      continue;
    }
    switch (n[i].type()) {
      default:
        nn = 0;
        warn("we're ignoring this");
        break;
    }
    orig->doSomethingWith(nn);
    delete nn;
  }

How do you like that switch with only a default branch? Pretty cool, huh? Almost is nice is the delete that only ever sees a NULL. Then when I realized doSomethingWith() is a no-op if it gets a NULL, I felt even luckier.

Look, if you're making such a big change to some code that you take out all the branches of a switch, isn't it worth a couple minutes to clean this up and show what is really happening? After all, the code really does this:

  for (i = 0; i < lim; i++) {
    if (someCondition(n[i])) {
      warn("you can't do that");
    }
    else {
      warn("we're ignoring this");
    }
  }

It probably makes sense to only give the second warning, since who cares if you can't do it, when really it would only be ignored anyway. And at that point you might wonder why you need to give the same warning lim times.

Ugly code kills. Kill ugly code.

Csh Stupidity

One of the great things about the Linux era, is that bash seems to be taking over as the preferred shell to use, instead of csh/tcsh. The projects at my current job began during the SunOS era, so when I started there a couple years ago I went with the flow and let the sysadmin set up my default as tcsh just like everyone else there uses. When I was getting started, no one had a reasonable .profile or .bashrc I could use, so I went with tcsh so I could use the tribal .cshrc.

In terminals, the first command I run is "bash"; most of my work is in an emacs shell window, so my .emacs says (setq shell-file-name "/bin/bash") -- actually there's an (if) in there to choose cygwin's bash if I happen to be on Windows. I do a lot of shell programming at the command line, and I simply can't live with csh-style programming.

A few days ago I was working on my .cshrc file, and I noticed how dim-witted csh's if-else handling is. The classic anti-csh diatribe is Csh Programming Considered Harmful, which is an entertaining read, but there's some insanity that isn't even covered in that worthy rant. Let me start by presenting a correct csh script:

setenv FOO hello
if ($FOO == hello) then
echo hi
else if ($FOO == goodbye) then
echo see ya
endif

Source that script, and the output is:

hi

Various typos can have effects on this script that range from annoying to silent but deadly. Here's an annoying one: put the second if on a different line from the else. The output is:

hi
else: endif not found.

Well, at least you got an error message telling you something is up, although requiring two keywords to be on the same line is a level of stupidity worthy of Tcl. What if you do the opposite, and have too many endifs? Well, the output is just "hi", with no error message. I suppose that doesn't bite too much, until later when you nest another if into the script, and it suddenly gets closed by one of your stealth extra endifs.

But here's a bad one. Take the original script. Add an else at the beginning, and an endif at the end. Seems like an else without an accompanying if should get a syntax error, but it doesn't. The extra endif -- which might have been there because we always had an extra one, but never got an error about it -- closes the weird else. And guess what? The script runs with no errors, and no output. The else is kind of a "if not true". How can this be a good thing? 

[Update 2013/11/21: Ha! Now this has actually happened to me, in almost the same way described here.  I had to add some functionality to an existing script at work.  It was a branch added near the top of an if with several branches. I mistakenly added an endif at the end of my new branch. The new functionality (and the branch above it) tested fine, no error reported, so I checked it in. A few days later someone noticed that the later branches weren't running (this script gets launched from a cron job, so the problem wasn't something that would get immediately noticed). The later branches -- starting with an else for which there was no active if -- were silently skipped. Grrrr...]

I did these experiments with csh and tcsh on RedHat. Avoid csh. Long live bash!

Makedepend: Error: Failed to Read ...

I complained a few months ago about makedepend not picking up dependencies, and gave a recipe for avoiding makedepend.

I'm stuck with it for a project, and a couple of times I've gotten a makedepend failure, with only this useless warning:

makedepend: error:  failed to read [some directory name]

The problem ended up being an #include somewhere, where the opening double-quote (") was missing. One time I accidentally had a single-quote at the beginning, closed with a double-quote; the other time a co-worker left off the opening quote, but did have the closing quote.

No help from makedepend on which file has the faulty include: it just gives up on the whole directory. Thanks, makedepend. Notice that if you leave off both quotes, or have both as single-quotes, makedepend is kind enough to give you a meaningful message. Or, if you open with a double-quote, and forget the closing quote, it exits without an error! Though I'm not sure if it gets the dependencies right in that case.

Do yourself a favor, use gcc's dependency-generating scheme, and never use makedepend again.

Extra Characters After Close-Brace

As noted previously, Tcl forces you to put open-braces "{" on the same line as the if or proc that they belong to, and forces you to use the atrocious "} else {" style.

That's appalling, but get this: it also requires a space to the right of the close-brace "}". That's just plain stupid.

Compounding the problem, the error message isn't something like "a space is required after a close-brace". Oh no. It says "extra characters after close-brace". And if the body of your if is very long, like this:

if {$tcl_is_stupid} {
  # ... many lines of code ...
  # Next line is the error
}else{
  i_eat_my_hat
}

then the error message is truncated so that you only see the part of the command that isn't broken, along with the line number of the "if", not the offending "else". As icing on the cake, in the case I was trying to help someone figure out, the error was at the top of one of Tcl's Icelandic Saga stacktraces, and the line number reported to be in error was with respect to the proc it was in, not the file.

Thankfully, Google led me to the answer, because Tcl sure didn't. I love the subtitle of that Tcl wiki page: Purpose: to discuss one of the few 'gotchas' in Tcl. Ha! What a crock!

Last Element in a Tcl List

Oh, thank you Tcl, for providing the special string end. That way I can get the last element in a list with:

  [lindex $mylist end]

What a relief! I was about to type:

  [lindex $mylist [expr {[llength $mylist] - 1}]]

And if I had had to type that, my head would have exploded. Fortunately, I got away with just a pulsing vein on my temple at the thought of a pseudo-keyword like end.

Open Your Eyes!

Here's a sweet piece of code (names have been changed to protect the guilty):

if (calculated_condition == some_test()) {
  // Ancient comment.
  // T intermediate = partial_value(val);
  // calculated_val = more_calculations(intermediate);
  // Merely elderly comment.
  // calculated_val = better_calculations(val);
  calculated_val = val;
}
else {
  calculated_val = val;
}

Good lord! Maybe if there wasn't a compulsion to comment out old code, it would have been clearer what was happening here, and this would have been replaced with the one-liner. Instead, a few years later I have to spend a couple minutes reading the whole thing, maybe pausing right at the beginning to figure out when the condition is met, only to find that none of it matters.

It's even more fun when several lines of code are commented with a single "/*", or the hated "#if 0". Then as you're grepping to figure out where something decrepit like more_calculations() is used, it looks like it's used in hundreds of places.

Delete dead code!

Tcl is Still a Toy Language

Let's extend the chain of complaints about Tcl.

6. Expressions sometimes have to be wrapped in [expr {}], but not always:

if { $x == "a" || $x == "b" } {
  return [expr {$x == "a"}]
}

7. Variable argument lists are handled with a special argument name: "args". It's not a keyword, just a variable treated differently from every other variable. That's just demented.

Stupid, stupid makedepend

One of the most frustrating, time-wasting debugging experiences you can have is when the code you thought you compiled didn't really get compiled. Or something you didn't even think about because someone else updated it didn't get compiled.

This is why the utility makedepend was introduced, so that compile dependencies get taken care of automatically, every time. So why is the Linux makedepend so stupid that it doesn't work every time?!?!

In the interest of speed, the Linux makedepend was written to parse include files only once per run (usually this is per directory). That's usually OK, but when the include file is preprocessed differently by different files (or multiple times in one file), it doesn't work. I thought the first rule of optimization was: make the common case fast, but make every case work.

Moral: use the dependency scheme described in the GNU make info pages. It goes like this:

# In the variables area:
sources = foo.cpp bar.cpp ...
OBJS = $(sources:.cpp=.o)

# In the targets area, somewhere after "all:"
ifneq ($(MAKECMDGOALS),clean)
include $(sources:.cpp=.d)
endif

%.d: %.cpp
  @(set -o pipefail; \
    $(CXX) -MM $(CXXFLAGS) $< \
     | sed 's,\($*\)\.o[ :]*,\1.o $@ : ,g' \
      > $@;) || $(RM) $@

Of course this assumes that CXX = g++. By the way, that little gem about not building the dependencies during "clean" is in the info pages, but not the page that describes the generic dependency rules. Yes, I have read every make info page.

The "pipefail" bit is my own little contribution; it saves you some headaches when the g++ command fails when you're not watching, because the sed command will succeed and allow make to continue ignorantly on its way. That's a bash option, so it might not work on platforms other than Linux.

Obfuscated Directory Names

I have to rant about some confusing directory/file/object names in a project I inherited at work. The guy who did it is a competent, experienced engineer. This isn't an attack on him or his work habits. If there's a moral to this rant, it's that good developers can make this kind of mistake, so the rest of us have to be extra careful not to make it.

Now, on to my rant. This C++ project -- for purposes of anonymity let's call it the "zoo" project -- interfaces with a Tcl interpreter. One confusing thing about the project organization is that it doesn't follow Rules 1.1 and 1.2 of my C++ coding standard: each source file might define many different classes. That compounds some other class naming issues, like the presence of classes with ridiculously similar names, like "ZooObj" and "ZooObject", or naming files after an abstract base class, but changing the capitalization: zooObject.hpp.

But the real problem has to do with directory names that don't describe their contents well, and that all sound the same. Here's the directory structure:

 ...
   zoo/
     tclApi/
     zooDb/
       zooDb/
       zooDbTcl/

The morpheme "db" here just means "I am defining a set of classes that model the zoo data". Well, no kidding, it's the zoo project. Things that go without saying should just not be said. In object-oriented design, responsibility for every function belongs to some class or another. You should never need special directories called "db" or "model" or "classes" or anything like that -- they don't add any information.

The next indication that something has gone wrong is that we have a directory called "zooDb/zooDb". That's one duplication that we could surely live without; you also have to wonder why there are two directories with "tcl" in the name. It turns out that one rationale for the directory structure is that there are a set of classes which will be compiled into a library with no dependencies on the larger project, so that it can provide a Tcl package to any interpreter. So maybe there's a method to this madness, we need the zoo/zooDb/zooDbTcl directory for that separate Tcl package code. Oops, turns out that's not the case, it's zoo/zooDb/zooDb that holds that. Arrrggggh!

What can we learn from this bad example? First, don't add directory structure where it's not needed. You probably need one directory for each object library you're building; any more than that just makes it confusing when someone is trying to walk through your code. Second, directory names and class names should not contain superfluous strings like "db", "model", or "class". Finally, don't add misleading strings -- like "tcl" in the example above -- that lead away from the directories or files that should be described with them.

If we apply all that, we end up with a much cleaner directory structure:

 ...
   zoo/
   zooTclPackage/

With that structure, when a new developer is looking for the classes that are in the dependency-free package, he knows just where to look. When he's looking for other zoo-project files, he knows where to look. It doesn't matter that some code interfacing with Tcl is mingled in a directory with stuff that knows nothing about Tcl -- like the food on your plate, it all gets mixed together when you eat it anyway. As an added benefit, the use of that flat hierarchy and putting every class in a separate header file will hopefully cut down on the temptation to create evil twins like zooObj and zooObject.

More Tcl Lunacy

Funny how when you start making a list, you might even leave off the thing that inspired you to make the list in the first place. The aggravation that led me to my previous rant didn't even make it into that post. So let's extend the "Tcl is a Toy" list a little:

4. Comments don't always begin with "#". At the beginning of a line, "#" is the comment character. But if you want to put a comment at the end of a line of code, you have to use ";#". I'm sorry, but that's ridiculous. So ridiculous that I had to start a blog just to rant about it, but when I went to post to the blog, I got sidetracked by three other ridiculous things.

5. Backslashes don't increase the line-number count. If you have some fancy scripts, or embedded Tcl, where you want to report the line number of certain actions, backslashes throw off the count. Why did you have backslashes in your script to begin with? Because Tcl is a toy language that would do something different if you started a new line there without the backslash. Argggh!

Tcl is a Toy Language

I just took over some Tcl code at work. What a toy language! The following are some things that are completely wrong about Tcl.

1. "else/elseif" has to be to the right of a brace. It's one thing if you like using this style of idiotic formatting for your if-else statements:

  if { $x == 0 } {
    do_zero()
  } else {
    do_non_zero()
  }

which hides the structure by putting the "else" in a different column than the "if". But for the language to require you to do such a stupid thing is, well, stupid. It's also bad that it requires you to cleverly put the open brace of the "if" on the same line. It's what I'd usually do anyway, but bad requirement.

2. $x is assigned to as "x". How insane is that? You have two names for the same thing, and you can silently do the wrong thing if you set $x "abc".

3. Whitespace is significant in multi-dimensional arrays. Excuse me? $matrix(1, 1) (with a space) is different than $matrix(1,1)?

I could go on, but that's enough ranting for today.