# Nested IF code in GUI Controls

I'm writing GUI controls and there are many places where there are many nested ifs checking for some result.

function TMyObject.GetCursor: TCursor;
begin
if CanDragX then
begin
if CanDragY then
Result := crSizeAll
else
Result := crSizeWE;
end
else if CanDragY then
Result := crSizeNS
else
if CanClick then
Result := crHandPoint
else
Result := crArrow;
end;


How would you format/rewrite this code?

I would do this:

function TMyObject.GetCursor: TCursor;
begin
if CanDragX and CanDragY then
Result := crSizeAll
else if CanDragX then
Result := crSizeWE
else if CanDragY then
Result := crSizeNS
else
if CanClick then
Result := crHandPoint
else
Result := crArrow;
end;


But really it is a matter of style and what is most readable to you (and the person who will maintain it).

• Thanks for your answer. I know this is a trivial case, but maybe I'm overlooking something, thats why I asked. – Kromster Aug 8 '11 at 8:03
• @Krom The only change I'd suggest to this answer is to put the "if CanClick then" on the same line with the preceeding "else" and adjust the following indentation accordingly. Like Martin said, it's just a matter of style. – jimreed Aug 8 '11 at 12:27

I'm not sure of what your language is. Instead of assigning to Result, I prefer mid-function returns in a case like this; then I don't need else.

function TMyObject.GetCursor: TCursor;
begin
if CanDragX && CanDragY then return crSizeAll;
if CanDragX then return crSizeWE;
if CanDragY then return crSizeNS;
if CanClick then return crHandPoint;
return crArrow;
end;


If that works in your language (VB?), I think that's much cleaner.

• It's Delphi, unfortunately there was no Delphi tag prior my question, I should have added that. In Delphi2009+ it would be "Exit (crSizeAll)" instead of "return crSizeAll". Thats a nice idea, unfortunately it does not work in Delphi7. +1 – Kromster Aug 9 '11 at 5:11

As there are only 8 possible results, you can put them into mapping array. Then function itself will be one-liner only.

const
CursorMap: array [boolean, boolean, boolean] // CanClick, CanDragX, CanDragY
of TCursor =               // see CanClick, CanDragX indexes values below:
(((crArrow, crSizeNS),     // false, false
(crSizeWE, crSizeAll)),  // false, true
((crHandPoint, crSizeNS), // true, false
(crSizeWE, crSizeAll))); // true, true

function TMyObject.GetCursor: TCursor;
begin
Result:= CursorMap[CanClick, CanDragX, CanDragY];
end;

• Clever way to make the function itself a single line. But you've moved all the complexity into CursorMap, and the need for comments should be proof enough that this is a bad idea. As you add more variables, the problem simply gets worse. – Disillusioned Jun 23 '12 at 11:08
• @CraigYoung, I just told that there is such way also. May it'll be useful for somebody. My personal favorite answer is with returns, by @CarlManaster – Serhii Kheilyk Jun 23 '12 at 15:20

In a method like this I like to make it clear up front that the default Result is crArrow and the rest of the routine is simply considering situations in which the default would be insufficient.

function TMyObject.GetCursor: TCursor;
begin
Result := crArrow;

if CanDragX and CanDragY then Result := crSizeAll
else if CanDragX then Result := crSizeWE
else if CanDragY then Result := crSizeNS
else if CanClick then Result := crHandPoint;
end;


This is somewhat similar to Carl's answer, but compensating for the fact that Delphi does not provide a single operation to both set Result and return.

A key difference to note here is that it is very important to be aware of the impact of else if vs simple if. For example, if you reverse the order of the checks, then you have to implement as follows:

if CanClick then Result := crHandPoint;
if CanDragY then Result := crSizeNS;
if CanDragX then Result := crSizeWE;
if CanDragX and CanDragY then Result := crSizeAll;


A more direct translation of Carl's answer might prove a little less error prone, albeit more verbose:

if CanDragX and CanDragY then
begin
Result := crSizeAll;
Exit;
end;
if CanDragX then
begin
Result := crSizeWE;
Exit;
end;
...


Even then I suspect mistakes would be easy to make.
Although I dislike both Sunny's and Sergiy's answers because they need comments for clarification, there's a lot to be said for explicitly mapping each permutation. Consider the following pseudo-code:

case [DragX, DragY, Click] of
[CanDragX, CanDragY, CanClick] : Result := crSizeAll;
[CanDragX, CanDragY, NotClick] : Result := crSizeAll;
[CanDragX, NotDragY, CanClick] : Result := crSizeWE;
[CanDragX, NotDragY, NotClick] : Result := crSizeWE;
[NotDragX, CanDragY, CanClick] : Result := crSizeNS;
[NotDragX, CanDragY, NotClick] : Result := crSizeNS;
[NotDragX, NotDragY, CanClick] : Result := crHandPoint;
[NotDragX, NotDragY, NotClick] : Result := crArrow;
end;


It would be wonderful if Delphi's enums and sets supported such syntax, but similar can be achieved using a self-documenting variation of Sunny's answer:

const
cCannotDo = 0;
cCanDragX = 1;
cCanDragY = 2;
cCanClick = 4;
var
LMouseOptions: Integer;
begin
LMouseOptions :=  Ord(CanDragX)*cCanDragX +
Ord(CanDragY)*cCanDragY +
Ord(CanClick)*cCanClick;
case LMouseOptions of
cCanDragX + cCanDragY + cCanClick : Result := crSizeAll;
cCanDragX + cCanDragY + cCannotDo : Result := crSizeAll;
cCanDragX + cCannotDo + cCanClick : Result := crSizeWE;
cCanDragX + cCannotDo + cCannotDo : Result := crSizeWE;
cCannotDo + cCanDragY + cCanClick : Result := crSizeNS;
cCannotDo + cCanDragY + cCannotDo : Result := crSizeNS;
cCannotDo + cCannotDo + cCanClick : Result := crHandPoint;
cCannotDo + cCannotDo + cCannotDo : Result := crArrow;
else
//Either raise error or return crArrow as default
end;
end;


However, the permutations can get unwieldy as more flags and even multi-value options are added. So personally I would go for my first option, and implement unit tests to run through all the cases of interest.

Using bitwise OR, and assuming this is some form of Pascal, but same applies to any languiage:

VAR:
res : integer

res := (ord(CanClick) * 4) OR (ord(CanDragY) * 2) OR ord(CanDragX);
case res of
0: Result := crArrow;      // 0 0 0
1, 5: Result := crSizeWE;  // 0 0 1 or 1 0 1
2, 6: Result := crSizeNS;  // 0 1 0  or 1 1 0
3, 7: Result := crSizeAll; // 0 1 1  or 1 1 1
4: Result := crHandPoint;  // 1 0 0
end;

• The need for comments should be an alarm bell screaming: Houston we have a problem! – Disillusioned Jun 23 '12 at 11:11