Should variable be initialized before call to function?

Question

When I call Functions to get a value, I usually initialize varible, in case function fails or doesn't return anything and I want to avoid dealing with uninitialized variable. I do the same for string, integer or any other type.

Example for integer variables:

vPropValue := 0;
vPropValue := GetPropValue(vObject,'Height');

IF vPropValue > 0 Then
...

this it the most common how I use it.

I know I could use:

If GetPropValue(vObject,'Height') > 0 Then
...

but with first example I avoid multiple calls to function, if I need result again later in the code.

Same for string (even though i know local strings are initialized to empty string, while integers are not an can hold any value)

vName := '';
vName := GetObjectName(vObject,'ObjectName');

IF Trim(vPropStrValue) <> '' Then
...

I know I could take steps to avoid duplicate value assignment,like making sure Function returns 0 if everything fails. But I have 100s of functions and I can't rely I never made a mistake how functions handle everything and I'm sure some don't return 0, if everything fails.

I'm trying to understand why this is not desirable practice and how to best avoid it.

EDIT

Here is example where function doesn't return proper value or 0:

function GetValue(vType:integer):integer;
begin
   if vType=1 then
    Result:=100
   else if (vType>2) and (vType<=9) then
     Result:=200;

end;

procedure TForm1.Button1Click(Sender: TObject);
var vValue:integer;
begin

  vValue:=GetValue(11);
  Button1.Caption:=IntToStr(vValue);

end;

In this case the value returned from function is some random number.

In this case the initialization appears to be valid approach. OR NOT?

EDIT 2:

As David pointed out in his answer, correct, there was a warning

[dcc32 Warning] Unit1.pas(33): W1035 Return value of function 'GetValue' might be undefined

but, I ignored it, for no reason, just didn't look there. As it let me compile it, I thought it was OK. So, I did look for the warning and I 'fixed' quite a few functions that had similar issue, because of all IFs Result might not have been defined.

EDIT 3 & CONCLUSION:

I hope it adds to the scope of question and explanation:

Maybe an example of another twist that I use in most of my Functions, would also explain why I thought my initialization of variable was needed, is that I wasn't sure my Functions would behave correctly all the times, especially in case of nested function. Mots of them still are set like this:

function GetProperty(vType:integer):integer;
begin
  Try
    if vType = 99 then
      Result:=GetDifferentProperty(vType)// <-- Call to another Function, that could return whatever...
    else
    begin
       if vType=1 then
          Result:=100
       else if (vType>2) and (vType<=9) then
         Result:=200;
    end;
  except
  end;
end;

Now I'm addressing these Try Except End; but some functions are 10 years old and to expect them to work 100%, based on my experience back then, is not something to rely on.

As the only developer on this project, I assume I should trust my functions (and he rest of my code), but I can't imagine in multiple developers environment that all function are set up properly.

So my conclusion: since I didn't take care of the basics - properly designed Functions, I need to have all these checks (variable initialization, Try Except lines..) and probably some other unneccessary stuff.

Dysthymia

Re: edit #1 - the initialization is a valid approach but should be moved INSIDE the function. pastebin.ca/3272345 – Nuke

I almost dismissed this as yet another duplicate of variable initialisation. But this was in fact a good question. You put in appropriate effort; and there are some valuable insights to be gained, even for experienced Delphi developers. +1 – Burgas

There are some RTL functions that raises an exception when the input is wrong. Example StrToInt(). To use them correctly, you are supposed to use a try/except frame, where you handle the exception case. In the exception handler you could set the value after the function call (or do something completely different). Generally you should avoid this construction if there are alternatives. Code can be hard to read and exceptions can decrease performance. In the example given, alternatives like StrToIntDef or TryStrToInt is to prefer. – Sabbatical

I don't understand the swallow all exception handler in your latest edit. That's a really bad practise. Btw, I'm impressed with your desire to tidy up your code. That's a very good sign! – Steamy

your edit 3 function is still wrong as it would give garbage values for some input values, like garbage would be given when vType=2. You should make default value INSIDE the function like I described in my link above. See updated sample pastebin.ca/3272690 – Nuke

@Arioch'The Correct, Edit 3 is example of my current 'style' of majority of my functions. With the knowledge in these answers I will start making changes in 100s of them. Pastebin.ca service seems to be down right now... – Dysthymia

@MikeTorrettinni grab it from google! webcache.googleusercontent.com/… – Nuke

Answer 1

Assuming that vPropValue is a local variable then this code

vPropValue := 0;
vPropValue := GetPropValue(vObject,'Height');

is indistinguishable from

vPropValue := GetPropValue(vObject,'Height');

A simpler example might be like so:

i := 0;
i := 1;

What is the point of assigning 0 to i and then immediately assigning 1 to i? So, you would surely never write that. You would write:

i := 1;

In your code, at the top of this answer, you assign twice to the same variable. The value assigned in the first assignment is immediately replaced by the value assigned in the second assignment. Therefore, the first assignment is pointless and should be removed.

The second example is a little more complex. Assuming that your functions are written correctly, and always assign to their return value, and that vName is a local variable, then

vName := '';
vName := GetObjectName(vObject,'ObjectName');

is indistinguishable from

vName := GetObjectName(vObject,'ObjectName');

The reason why I have added an extra proviso relates to a quirk of the implementation of function return values, discussed below. The difference between this case and the case above is the return value type. Here it is a managed type, string, whereas in the first example the type is a simple Integer.

Again, given the proviso about the function always assigning to the return value, the first assignment is pointless because the value is immediately replaced. Remove that first assignment.

---

Regarding the function in your edit, the compiler will warn you of its erroneous implementation if you enable hints and warnings. The compiler will tell you that not all code paths return a value.

function GetValue(vType:integer):integer;
begin
  if vType=1 then
    Result:=100
  else if (vType>2) and (vType<=9) then
    Result:=200;
end;

If neither condition is met, then no value is assigned to the result variable. This function should be:

function GetValue(vType:integer):integer;
begin
  if vType=1 then
    Result:=100
  else if (vType>2) and (vType<=9) then
    Result:=200
  else
    Result:=0;
end;

I cannot stress how important it is that you always return a value from a function. In fact it is a terrible weakness that Delphi even allows your function to be compiled.

---

The reason that your double assignment sometimes appears useful to you is due to a quirk of of the implementation of function return values in Delphi. Unlike almost all other languages a Delphi function return value for certain more complex types is actually a var parameter. So this function

function foo: string;

is actually, semantically, the same as this:

procedure foo(var result: string);

This is a really odd decision made by the Delphi designers. In most other languages, like C, C++, C#, Java etc., a function return value is like a by-value parameter passed from callee to caller.

This means that you can, if you wish to be perverse, pass values into a function via its return value. For instance, consider this code:

// Note: this code is an example of very bad practice, do not write code like this

function foo: string;
begin
  Writeln(Result);
end;

procedure main;
var
  s: string;
begin
  s := 'bar';
  s := foo;
end;

When you call main, it will output bar. This is a rather strange implementation detail. You should not rely on it. Let me repeat myself. You should not rely on it. Do not get in the habit of initializing return values at the call site. That leads to unmaintainable code.

Instead follow the simple rule of ensuring that the function return value is always assigned by the function, and never read before it has been assigned.

---

More detail on the implementation of function return values is provided by the documentation, with my emphasis:

The following conventions are used for returning function result values.

• Ordinal results are returned, when possible, in a CPU register. Bytes are returned in AL, words are returned in AX, and double-words are returned in EAX.
• Real results are returned in the floating-point coprocessor's top-of-stack register (ST(0)). For function results of type Currency, the value in ST(0) is scaled by 10000. For example, the Currency value 1.234 is returned in ST(0) as 12340.
• For a string, dynamic array, method pointer, or variant result, the effects are the same as if the function result were declared as an additional var parameter following the declared parameters. In other words, the caller passes an additional 32-bit pointer that points to a variable in which to return the function result.
• Int64 is returned in EDX:EAX.
• Pointer, class, class-reference, and procedure-pointer results are returned in EAX.
• For static-array, record, and set results, if the value occupies one byte it is returned in AL; if the value occupies two bytes it is returned in AX; and if the value occupies four bytes it is returned in EAX. Otherwise, the result is returned in an additional var parameter that is passed to the function after the declared parameters.

Steamy

Very good insight, thank you for detailed explanation. Correct, the warning was there, but I didn't notice it. – Dysthymia

Interesting implementation of function return, indeed, thanks for pointing that out. That makes me wondering: if we call function like a procedure, without assigning result somewhere, does compiler create some dummy variable at caller side and converts, for example, "showModal" to "_somevar:=showModal" so there is a place where to put result? – Foretime

@Yuriy The compiler doesn't create a dummy variable when you call functions as procedures. It actually works the other way around: a function always returns a value in a special location. If you call it as a procedure that returned value is simply ignored and nothing else happens (apart from managed type clean-up). However, if you call the function as a function, the compiler copies the return value into your chosen location after the function returns. (NOTE the compiler can perform certain optimisations which change things in special cases, but this is the gist of it.) – Burgas

@CraigYoung which basically means the same. The compiler does create a dummy variable, just not always copies the value out of that dummy variable. And that implicit dummy variable ( for managed types like interfaces, dyn-arrays, strings) at caller site is what provides for RAII-style tricks like deltics.co.nz/blog/posts/404 – Nuke

@CraigYoung I was economical with the details in earlier answers. Have a read of the doc quote now included. Note especially the part about the extra var parameter. – Steamy

@YuriyAfanasenkov with your particular example ( ShowModal ) though you use int32 return type and there is nothing for compiler to do: this type - just check Delphi default calling convention - is returned via EAX register of CPU, which is always there for that purpose and nothing needed to be additionally allocated. – Nuke

@YuriyAfanasenkov There are indeed dummy variables, but only for certain complex types, as described in the doc quote I added to the answer. – Steamy

@Arioch'The What I tried to emphasise was that the dummy is not created for the special case of calling "function as a procedure", but that it's always there. – Burgas

@DavidHeffernan Delphi docs are obsolete as usual.... Not a single mention of RAX in AMD64 compiling mode :-/ – Nuke

@Arioch'The True. The platform ABI is followed, apart from some exceptions. Notably the handling of return values! – Steamy

@CraigYoung A dummy variable is created in the cases where the return value is passed as a var parameter. – Steamy

@DavidHeffernan that is one of the reasons I believe a person should try some assembler programming for Delphi - to understand the basics of the scaffolding. He can not just "memorize the docs" for the docs are poor. One better understands what is going on and be able to look and see with debugging tools when his expectation failed. // personally when I had to do some asm for xe2 x64 I exactly made purepascal functions and looked at the calling/exiting processes, 'cause the compiler not the missing docs are the ultimate authority here – Nuke

@DavidHeffernan w.r.t. "is a terrible weakness that Delphi" - we should understand that flow analysis in Delphi is very limited. Classic problem: ValueFlag := false; ..... if ... then begin ... HeavyComplexValue := ...; ValueFlag := true; end; ..... if ValueFlag then .... using(HeavyComplexValue); - and here compiler would tell as "non initialized value" which is WRONG. So if you prefer to turn this particular warning into a hard error - you can do it in your IDE/project options. But given Delphi limitations I would not gamble upon compiler ALWAYS determining if result value is set or not – Nuke

@David Yes your addition of the return conventions is a very useful clarification of when a dummy variable is "created" and when a fixed location is used. All in all I'd say this question has turned out to be much more interesting than it would seem at first glance. – Burgas

@CraigYoung Indeed. Thanks for prompting me to be more precise. – Steamy

So, if int32 are returned in EAX, then if function didn't assign anything to result, there can be some trash in EAX which will be copied to variable when we call value:=func(...) and in that case there is really no point in initializing value before, it won't save it from storing garbage after calling func(...). The only way when initializing matters is when result is passed as additional var parameter. – Foretime

@YuriyAfanasenkov It's more complex than that. The value that happens to be in EAX could be well defined. It's whatever happened to be there before the call was made. Often it will be the first parameter passed to the function. So it might not be trash and the code may well behave as intended, quite by accident. The program can then break due to small changes like compiler optimisations on/off. – Steamy

@DavidHeffernan ...or like switch to AMD64 platform where Delphi would switch to "RCX, RDX, R8, R9" registers and thus would not fill RAX/EAX with the in-parameter. S basically Yuriy is correct, in his example EAX really contain garbage on exit, because garbage is anything that you did not intentionally put there. As long as you was not writing hackish quirks-dependent code the transfusion of in-parameter into ret-value is a non-intended accident, thus a garbage. – Nuke

@Arioch'The My point is just that this garbage can, sometimes, accidentally be well-defined in a way that disguises the broken implementation of the function – Steamy

Answer 2

The following code (A)

vPropValue := 0;
vPropValue := GetPropValue(vObject,'Height');

is indistinguishable from (B)

vPropValue := GetPropValue(vObject,'Height');

The question of whether GetPropValue is "written correctly" is totally irrelevant.

Let's consider what happens even if you did write GetPropValue incorrectly E.g.

function GetPropValue(AObject: TObject; AStr: String): Integer;
begin
  if AStr = 'Hello' then Result := 5;
end;

As you know when the input AStr is anything other than "Hello" the result of the function will be pretty much random. (For the sake of the discussion, lets assume it will return -42.)

Code block (A) will do the following:

• Set vPropValue to 0
• Then set vPropValue to - 42

Code block (B) will simply set vPropValue to - 42 immediately.

TIP: There is no point in writing a wasteful line of code just because you're worried you might have made a mistake in a function you call.
First, As David points out, you can avoid many mistakes simply by paying attention to your compiler hints and warnings. Second, that sort of "paranoid" coding simply leads to more wasteful code because now you have to start considering invalid values as possible results.
This becomes worse when one day your "safe-value" is actually a valid value. E.g. how would you tell the difference between "default 0" and "correctly returned 0"?

Don't make programming artificially difficult by bloating code with unnecessary redundancies.

---

Side Note

There are a couple of special situations where the code can behave differently. But you should in any case avoid the designs that lead to those situations because they make it much more difficult to maintain the code.

I mention them purely for the sake of completeness, the advice above still stands.

1) if vPropValue is implemented as a property, the setter could have side-effects that cause different behaviour. While there's nothing wrong with properties, when they do unexpected things you have a serious problem on your hands.

2) if vPropValue is a field on the class (or worse a global variable), then (A) and (B) could behave differently but only if GetPropValue raises an exception. This is because the exception would prevent the result from being assigned. Note this is something to avoid except in special cases because it does make it more difficult to reason about what your code is doing.

And in fact, this is something that makes it so much more important to avoid the redundant initialisations. You want your special case code to look stand out from the rest.

Burgas

The question of whether GetPropValue is "written correctly" is totally irrelevant. True for unmanaged result types that come back in registers. I missed that nuance in my early versions, but have now corrected that omission. Consider managed result types where the result value is passed as a var parameter. Specifically the doc quotes in my answer, and the example code which passes a value in to a function via its result value. – Steamy

Regarding side note 2, there is another case to consider, beyond exceptions. That is if the variable is visible to another thread. I've omitted all these nuances because they are edge cases, which is why you put them as a side note I am sure. – Steamy

@David Yes, that's exactly they're simply a side note. OP's code doesn't show that the variables are local so I figured it's worth raising. And I did consider mentioning threads but decided not to because I felt that would be a bit of scope creep. – Burgas

that sort of "paranoid" coding simply leads to more wasteful code I totally agree and upvote :) – Materse

Thank you @CraigYoung, now, reading all this great content in answers, I agree it should be irrelevant if the function is written correctly or not, but as I noted in Edit 3, it was a combination of not knowing how things work and understanding (or expecting?) that the function could fail and maybe will not return any value. – Dysthymia

Answer 3

Scavenging my advices from top-level comments in case Pastebin fails

function GetValueUpdate3(vType:integer):integer;
begin
// with SOME types like Integer you can use Pascal case-block instead of error-prone many-ifs-ladder

  case vType of 

    99:   Result:=GetDifferentProperty(vType);// <-- Call to another Function, that could return whatever...
    1:    Result:=100;
    3..9: Result:=200;

    else Result := 12345; // initialization with safe default value for illegal input like vType=2  

  end; // case-block
end;

function GetValueUpdate3e(vType:integer):integer;
begin
  case vType of

    99:   Result:=GetDifferentProperty(vType);// <-- Call to another Function, that could return whatever...
    1:    Result:=100;
    3..9: Result:=200;

    else raise EInvalidArgument.Create('prohibited value - GetValue( ' + IntToStr(vType) +' )' );
      // runtime eror when vType = 2 or any other illegal input
  end;
end;


function GetValueUpdate1(vType:integer):integer;
begin
  Result := 12345; // initialization with safe default value;

  if vType=1 then Exit(100); // special value for special case #1

  if (vType>2) and (vType<=9) then Exit(200); // special value for special case #2

  // exit with default value
end;

procedure TForm1.Button1Click(Sender: TObject);
var vValue:integer;
begin

  vValue:=GetValue(11);
  Button1.Caption:=IntToStr(vValue);

end;

// http://stackoverflow.com/questions/33927750

Dorsum

Thank you, especially to point out the use of 'Exit(Code)', I only use it once in all of my code. Time to rethink this also. – Dysthymia

It's not such a great idea. You end up with a mix of exit(.._) and Result := which is, in my view, often more confusing than beneficial – Steamy

Exit(code) is a shortcut for special case instant exit. Shortcut for tedious begin/Result:=/exit/end To use it or not is a personal option. Depending on the structure of function. If function has no special cases, then there is no point to use this parametrized exit. Even the very fact that Delphi introduced the Result variable - is it good or not? It provides for Result being used as a regular variable storing intermediate values. Maybe just using Delphi-invented Result variable is bad per se and we should go back to Pascal ? – Nuke

@DavidHeffernan below there is an answer by Alexander Balzer - and he does not use "special cases" idea, so when he codes the function similar functionally to my Upd1 function he structures it as an if-ladder with all options having similar semantical category. And "default value" goes last in line there (which personally I do not like). And my function uses different semantics, it uses PRE-assignment of default value (which might be less efficient CPU-wise) and a set of isolated special cases rather than chained ladder. But functionally they are the same - it is a matter of semantics – Nuke

It's just my personal opinion that exit(...) doesn't bring much to the table. – Steamy

Answer 4

You can use Assertions to validate if your input is correct. Assertions help a lot in finding logical errors in your code.

Using assertions forces you to think about valid and invalid input data and helps writing better code. Enabling and disabling assertions at compile time is done via the \$C or \$ASSERTIONS compiler (global switch)

In your example the function GetValue could use an assertion as follows:

function GetValue(vType:integer):integer;
begin
   Assert((vType >= 1) and (vType <=9), 'Invalid input value in function GetValue');

   if vType=1 then
    Result:=100
   else if (vType>2) and (vType<=9) then
     Result:=200
   else Result := 0; // always catch the last else!
end;

In addition every if-statement should catch the final else! (In my opinion ALWAYS!)

Benetta

Thank you, I already learned so much from all these answers here and now about Assert. – Dysthymia

He-he-he. You still missed the invalid vType=2 case in your assertion. Mike, those assertions might introduce you to general concepts such as c2.com/cgi/wiki?DefensiveProgramming /// c2.com/cgi/wiki?DesignByContract /// c2.com/cgi/wiki?DesignByContractAssertionsVsUnitTestsVsTypes – Nuke

Good point, Arioch 'The 9. That's why it is important to catch always the last ELSE ;-) I am a little bit surprised that it took so long until someone pointed out the incomplete assertion :-) – Benetta

@AlexanderBalzer last else or always set Result first, right? Result := 0; as a first line... – Dysthymia