Widgets How Not to Code: Passing function results directly as parameters

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:

  TData = record
    a, b: integer;

  TMoreData = record
    c, d: string

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:

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

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. 

blog comments powered by Disqus

My Book

A Pithy Quote for You

"Everybody is a genius. But if you judge a fish by its ability to climb a tree, it will live its whole life believing that it is stupid."    –  Albert Einstein

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.