Amazon.com Widgets How Not To Code

How Not to Code #3: Compound ‘with’ Statements

By Nick at November 03, 2012 04:24
Filed Under: Delphi, How Not To Code, Software Development

Seriously, think about this:

  with (Sender as tStringGrid), Canvas do

I’m doing some string grid work, and ran across this code out on the web.  Wow.  It does what I want, but really?  It was code on a page with a date of like 1998, so at least we could say it was before many Delphi developers knew better. But still….. that is just awful. 

Here’s the truly hideous part – if I comment the line out,  the code still compiles!  Yikes!

How Not To Code #2: Don’t Use Boolean Method Parameters

By Nick at August 04, 2012 04:16
Filed Under: Delphi, How Not To Code

Okay, let’s start off with a question.  What do you think this little code snippet does?

begin
  ProcessCustomerInput(False);
end;

Well, I’ll bet your first guess is that it processes customer input.  Good guess!  But what the heck does that False there mean?  It’s a parameter, and presumably it means something is, well, false, or that you don’t want the ProcessCustomerInput to do something, but how can you know?  You can’t.

Using Boolean parameters means that lose information for the reader of your code.  A Boolean parameter communicate nothing about the purpose or meaning of the parameter being passed.  Is False good or bad?  Safe or not safe?  Who knows?  Just as above, you can’t figure out at all what the parameter is supposed to mean or do if all you see is True or False

So if you see that in code, the first thing you’ll probably do is to go to the method declaration, and if the parameter is well named, you might figure out what the parameter does:

procedure ProcessCustomerInput(aKeepDuplicates: Boolean);

So there you go, now you know what the Boolean parameter does – apparently it tells you whether to keep the duplicates or not.  That’s great, but the original coder may not always be so kind and clear.  So if you must use a Boolean parameter, at least make the parameter name descriptive. 

Okay, I take it back – don’t actually ever use a Boolean parameter.  Instead, here’s what I suggest would make for much clearer code:

type
  TKeepDuplicates = (DoKeepTheDuplicates, DoNotKeepTheDuplicates);

procedure ProcessCustomerInput(aKeepDuplicates: TKeepDuplicates);
begin
  ....
end;

and that way you can call

begin
  ProcessCustomerInput(DoNotKeepTheDuplicates);
end;

And then your code is eminently readable and clear without having to look up the method declaration.

In addition, this way of doing things is expandable.  If your business rules change, and a third way of processing customer input appears, your code is ready.  With a Boolean parameter, you are stuck with the two options of True and False.

Easy to read, clear, and ready for the future.  Just like code should be.

How Not to Code: Passing function results directly as parameters

By Nick at March 11, 2012 12:27
Filed Under: Delphi, How Not To Code

I think I’ll start a new series, and a new blog category, called “How Not To Code”.  In it, I’ll try to highlight those little things that we’ve tended to do over the years that, well, are tempting for expediency, but probably not something we should be doing.  There are a lot of “Here’s the right way to do things”, so this series will concentrate on the “what not to do”.  That will inevitably lead, of course, to a discussion of “What to do”, but that’s ultimately the point, I suppose.

Today’s topic is “Don’t directly passing function results as parameters to other functions and procedures”. 

For instance, consider the following code:

type
  TData = record
    a, b: integer;
  end;

  TMoreData = record
    c, d: string
  end;

function GetSomeData(aInteger: integer; aString: string): TData;
function GetMoreData(aBoolean: Boolean; aInteger: integer): TMoreData;

procedure ProcessData(aData: TData; aMoreData: TMoreData);

Then, imagine that you wanted to call ProcessData with the result of  calls to GetSomeData  and GetMoreData.   It would be mighty tempting to simply call:

ProcessData(GetSomeData(aIntVariable, 'ocelot'), GetMoreData(True, AnotherIntVariable));

After all, think of all the typing you’ll save!

You should avoid that temptation, because this way of doing things presents a few problems:

  • It’s really hard to read.
    • A line of code that does more than one thing requires more brain power to read that does code where each line does one thing.  In this case, you have to convince your brain to parse all those parentheses.  Before you can figure out what the code is really doing, you have to parse two function calls inside of a procedure call.  Using your brain to parse unnecessarily complex semantics is a waste of effort.
    • You can’t be sure by merely reading the code what the types of the parameters are that are being passed to ProcessData.  This means that you have to look up the parameter types to gain full understanding.  If the variables were declared locally, then it would be easy to see their types.
  • It’s really hard to debug
    • There are no intermediate results to check in the debugger.
    • Stepping into the code requires that you step into the calls to GetSomeData and GetMoreData, when that might be something that you don’t want to have to do.

Instead of trying to compress everything into a single line of code, you probably would do something like this, instead:

var
  TempData: TData;
  TempMoreData: TMoreData;
begin
  TempData := GetSomeData(aIntVariable, 'ocelot');
  TempMoreData := GetMoreData(True, AnotherIntVariable);
  ProcessData(TempData, TempMoreData);
end;

The above code solves the aforementioned problems – it is easier to parse in your head and you can easily see the data values in the debugger before they are passed to ProcessData

Some good heuristics that fall out of this:

  • If you have to stare at the code for a moment just to parse it in your head before you even figure out what is going on, then you have a problem.
  • If you have a hard time figuring out where to put a breakpoint to get at the debugging information that you really want, then you have a problem.
  • If you feel the need to create intermediate variables to improve your ability to debug code, then you probably should have put the intermediate variables there in the first place. 
  • All sorts of bad things happen when you try to be clever and save a few keystrokes by taking a code shortcut.  Remember, the future maintainer is the person you should be writing code for, not the compiler.
  • A line of code should do one thing and one thing only – if the explanation of what a single line of code does needs the word “and” to describe it, then it probably ought to be two lines of code.

Lesson for the Day:  Always pass variables, never direct function calls, as parameters to another procedure or function. 

My Book

A Pithy Quote for You

"Science is the belief in the ignorance of experts."    –  Richard P. Feynman

Amazon Gift Cards

General Disclaimer

The views I express here are entirely my own and not necessarily those of any other rational person or organization.  However, I strongly recommend that you agree with pretty much everything I say because, well, I'm right.  Most of the time. Except when I'm not, in which case, you shouldn't agree with me.