Which code you suggest?

Rajesh Anuhya

Code1:

   Boolean DoSomething(string\[\] values)
    {
        foreach (string s in values)
            if (s == "ABC")
                return true;
        return false;
    }

Code2:

Boolean DoSomething(string[] values)
{
bool retValue = false;
foreach (string s in values)
if (s == "ABC")
retValue=true;
return retValue;
}

in the above 2 codes which code you will suggest and why? waiting for your valuable comments. Thanks --RA

my Tip/Tricks[^] |Contact me[^]

_Damian S_

Personally I prefer the second option, as there's only a single return from the function. That said, there's a whole bunch wrong with the code... or at least not good practice. (Yes, I realise it's just an example.)

Quad skating his way through the world since the early 80's... Booger Mobile - My bright green 1964 Ford Falcon - check out the blog here!! | If you feel generous - make a donation to Camp Quality!!

NeverJustHere

The first will be faster, as it will exit as soon as finding "ABC" Alternatively,

return values.Contains("ABC")

Ingo

I don't want to see any of these running, because both are bad style. Inline-Statements, no brackets, bad string comparison. They are EVIL! :omg:

SharePoint Consultant and Developer at acocon Author of Primary ROleplaying SysTem War doesn't determine who's right. War determines who's left.

ZurdoDev

I prefer the second method. I try not to have multiple places where a function can return.

There are only 10 types of people in the world, those who understand binary and those who don't.

Dave Kreskowiak

On top of everything else, you're post isn't even appropriate for this forum!

A guide to posting questions on CodeProject[^]
Dave Kreskowiak

Ron Beyer

I'm a fan of early exit, so I'd go with the first one (assuming that's what this post is about). It doesn't really matter much in smaller routines like the one above, but when you have longer ones it can be difficult to follow retValue around the function to find out where you are really setting the return value. The first method is shorter because you can run a case through in your head without having to write down variables. I think the question boils down to Early Exit versus Single Exit. There's a lot of debate on the merits of both so I think your answers are going to be somewhat distributed between them. I've yet to hear a debate for single exit that I agree with over the merits of early exit...

OriginalGriff

I think that's an unnecessary restriction - I prefer to do all my validation code / user notification en mass at the top of a method, and exit immediately.

if (!userGotHisNameRight)
Report It
return
if (!userManganagedHisAddressOK)
Report It
return
if (...)
...
Actual work the method is supposed to do

The alternataive being:

if (!userGotHisNameRight)
Report It
else if (!userManganagedHisAddressOK)
Report It
else if (...)
...
else
{
Actual work the method is supposed to do
}

The universe is composed of electrons, neutrons, protons and......morons. (ThePhantomUpvoter)

ZurdoDev

Ya, I know. Some people like it and some don't. Generally speaking the amount of time saved by returning in various places is negligible.

There are only 10 types of people in the world, those who understand binary and those who don't.

Sentenryu

ryanb31 wrote:

Generally speaking the amount of time saved by returning in various places is negligible.

Generalization is Bad. what would you say if there was a database access on that loop? or a file read/write? a service call? are those so rare that they can't be considered "general"? I'm ok with having one single return point, provided that the method doesn't do any significant work. the general rule should be to exit as soon as you finish the work or realize there's nothing more to be done.

I'm brazilian and english (well, human languages in general) aren't my best skill, so, sorry by my english. (if you want we can speak in C# or VB.Net =p) "Given the chance I'd rather work smart than work hard." - PHS241 "'Sophisticated platform' typically means 'I have no idea how it works.'"

ZurdoDev

But you're missing the fact that you can exit a loop when you find what you need. You don't have to continue processing.

There are only 10 types of people in the world, those who understand binary and those who don't.

OriginalGriff

What if you have two nested loops? It's a lot harder to exit them both...

The universe is composed of electrons, neutrons, protons and......morons. (ThePhantomUpvoter)

ZurdoDev

Quote:

What if you have two nested loops?

You can't get out of the matrix. :) I didn't say there was never a reason for returning from multiple places. I just said I prefer not to.

There are only 10 types of people in the world, those who understand binary and those who don't.

OriginalGriff

Trust me, I took the red pill a loooong time ago!

The universe is composed of electrons, neutrons, protons and......morons. (ThePhantomUpvoter)

ZurdoDev

Quote:

it can be difficult to follow retValue around the function to find out where you are really setting the return value.

That's funny. That's the same reason people usually argue for single exit, in that it is hard to figure out why some code isn't running because it turns out there was a return statement earlier that you hadn't noticed. :) To each his own.

There are only 10 types of people in the world, those who understand binary and those who don't.

ZurdoDev

:)

There are only 10 types of people in the world, those who understand binary and those who don't.

Sentenryu

the 2 loops are a good example, but for me is easier to track multiple exit points than it is to track changes on a variable. there may be a hundred variables involved, the return value can get reset, millions of things can happen, but when there's a return statement, i know it's over. whatever i've at that point is the result, i can track individual cases one at a time. for me it's really easier to track execution paths this way, is it different for you? the ammount of different opnions when we talk about code is just funny ;P opnions are the oposite of highlanders, there can never be only one.

I'm brazilian and english (well, human languages in general) aren't my best skill, so, sorry by my english. (if you want we can speak in C# or VB.Net =p) "Given the chance I'd rather work smart than work hard." - PHS241 "'Sophisticated platform' typically means 'I have no idea how it works.'"

CodeHawkz

First of all, the second code should have a 'break' as follows. Otherwise, it just continues to loop pointlessly, even if it find "ABC" in the first item.

Boolean DoSomething(string[] values)
{
bool retValue = false;
foreach (string s in values)
{
if (s == "ABC")
{
retValue=true;
break;
}
}
return retValue;
}

Personally, I choose the this method over your first method, since it has a single point of return to the method. About the performance, both versions should be identical as this method would only execute 2 steps extra.

BubingaMan

So, this is certainly choosing between the lesser of 2 evils :wtf: If I HAD to choose one of both, the first example would be it, even if only for performance reasons - since the loop breaks when ABC was found. But having said that... No. Just... No. :-D

Reelix

You left out the ;

-= Reelix =-