Guess His Experience Level

[Tristan Rhodes]

Pete O'Hanlon wrote:

Tristan Rhodes wrote: * Only one return per function (no short circuit conditions), leading to horrendous nesting. Not bad advice - just keep your functions short or you're in trouble.

I'd rather see this

MenuItemNode child = tvMenu.SelectedNode as MenuItemNode;

if (child == null)
return;

MenuItemNode parent = child.Parent as MenuItemNode;

if (parent == null)
return;

MenuItemNode swap = child.PrevNode as MenuItemNode;

if (swap == null)
return;

//Code Here

Than This

MenuItemNode child = tvMenu.SelectedNode as MenuItemNode;

if (child != null)
{
MenuItemNode parent = child.Parent as MenuItemNode;

  if (parent != null)
  {
        MenuItemNode swap = child.PrevNode as MenuItemNode;

        if (swap != null)
        {
              //Code Here
        }
  }

}

I just find the second option ugly, and it really isn't any easier to read.

------------------------------- Carrier Bags - 21st Century Tumbleweed.

modified on Sunday, April 5, 2009 7:33 AM

[jayart]

I think the same. But no one in my team thinks this way and they have managed to make this as a coding rule.

[Dan Neely]

Will they let you get away with stuffing all the nested initialization/validation crap into a separate method? eg something like this (I know it needs a bit more fiddling to compile):

bool IntializeAndvalidate (out MenuItemNode child, out MenuItemNode parent, MenuItemNode swap)
{
MenuItemNode child = tvMenu.SelectedNode as MenuItemNode;

  if (child != null)
  {
        MenuItemNode parent = child.Parent as MenuItemNode;

        if (parent != null)
        {
              MenuItemNode swap = child.PrevNode as MenuItemNode;

              return  (swap != null);
        }
  }

}

if (IntializeAndvalidate (child, parent, swap)
{
// do stuff
}

Today's lesson is brought to you by the word "niggardly". Remember kids, don't attribute to racism what can be explained by Scandinavian language roots. -- Robert Royall

[Luc Pattyn]

The official solution to avoid multiple returns is throwing exceptions all over the place, especially on input validation. :)

Luc Pattyn [Forum Guidelines] [My Articles]

---

- before you ask a question here, search CodeProject, then Google - the quality and detail of your question reflects on the effectiveness of the help you are likely to get - use the code block button (PRE tags) to preserve formatting when showing multi-line code snippets

---

[Dan Neely]

:rolleyes:

Today's lesson is brought to you by the word "niggardly". Remember kids, don't attribute to racism what can be explained by Scandinavian language roots. -- Robert Royall

[kfoster]

The 'one return per function' rule is a bit old school (early 90's :) ). IBM had it as a requirement when I was contracting there. It's not so much about style and readability as it is about introducing subtle bugs. If a maintenance programmer has to put some code in the function that has to take place before the function ends, he may place it right before the final 'return' near the end of the code block. And if he was working fast, or just working stupid he may not know of the other return points. Anyway, OOP guidelines of very short (7 +- 2) meaningful lines of code per method made this a moot point.

[Tristan Rhodes]

I've used that method before, but i was always told by my teacher to not use exceptions to control application flow, so it's something I'm reluctant to do unless I'm out of options. Still, if you want a short circuit validation routine with no input or output, guess that's the only way. What's the performance hit like for exceptions in ASP.Net? I thought exceptions were super bad at crippling servers?

------------------------------- Carrier Bags - 21st Century Tumbleweed.

[Luc Pattyn]

Hi, there is no performance hit for testing and throwing exceptions; what is slow is catching exceptions, especially so for the first one caught inside Visual Studio. But then exceptions should be used to deal with exceptional circumstances only, which does *not* mean they should be used rarely. It would be a bad idea to use them like so:

int[] numbers=new int[100];
for (int i=0; ; i++) numbers[i]=i; // break with IndexOutOfRangeException

:)

Luc Pattyn [Forum Guidelines] [My Articles]

---

- before you ask a question here, search CodeProject, then Google - the quality and detail of your question reflects on the effectiveness of the help you are likely to get - use the code block button (PRE tags) to preserve formatting when showing multi-line code snippets

---

[Pete OHanlon]

You could always do this as:

if (child != null && child.Parent != null && child.PrevNode != null)
{
}

"WPF has many lovers. It's a veritable porn star!" - Josh Smith

As Braveheart once said, "You can take our freedom but you'll never take our Hobnobs!" - Martin Hughes.

My blog | My articles | MoXAML PowerToys | Onyx

[OriginalGriff]

I sort of agree, but rather than:

MenuItemNode child = tvMenu.SelectedNode as MenuItemNode;
if (child == null)
return;
MenuItemNode parent = child.Parent as MenuItemNode;
if (parent == null)
return;
MenuItemNode swap = child.PrevNode as MenuItemNode;
if (swap == null)
return;
//Code Here

I would prefer:

MenuItemNode child = tvMenu.SelectedNode as MenuItemNode;
if (child == null)
{
return;
}
MenuItemNode parent = child.Parent as MenuItemNode;
if (parent == null)
{
return;
}
MenuItemNode swap = child.PrevNode as MenuItemNode;
if (swap == null)
{
return;
}
//Code Here

Just to remind me if I add a statement before the return.