473,324 Members | 2,313 Online
Bytes | Software Development & Data Engineering Community
Post Job

Home Posts Topics Members FAQ

Join Bytes to post your question to a community of 473,324 software developers and data experts.

Question about return style

I was just wondering if there is a "best" choice from the following
couple of ways of returning a value from a method:

1)
private HashAlgorithm GetSpecificHashAlgorithm(string hashString){
if (hashString == "MD5") {
return System.Security.Cryptography.MD5.Create();
} else {
return System.Security.Cryptography.SHA512.Create();
}
}

or

2)
private HashAlgorithm GetSpecificHashAlgorithm(string hashString){
HashAlgorithm hash;
if (hashString == "MD5") {
hash = System.Security.Cryptography.MD5.Create();
} else {
hash = System.Security.Cryptography.SHA512.Create();
}
return hash;
}

Is there a general opinion on which is better style?

Thanks,
Matt
Jun 27 '08 #1
12 1455
Unless you have additional code that might want to use the local "hash"
variable, I don't think it makes much difference. However, you might want
to pay attention to what to do is somebody calls your method and passes
"hamburger" as the parameter...
Peter
"Matt B" <ro**********@gmail.comwrote in message
news:29**********************************@s21g2000 prm.googlegroups.com...
>I was just wondering if there is a "best" choice from the following
couple of ways of returning a value from a method:

1)
private HashAlgorithm GetSpecificHashAlgorithm(string hashString){
if (hashString == "MD5") {
return System.Security.Cryptography.MD5.Create();
} else {
return System.Security.Cryptography.SHA512.Create();
}
}

or

2)
private HashAlgorithm GetSpecificHashAlgorithm(string hashString){
HashAlgorithm hash;
if (hashString == "MD5") {
hash = System.Security.Cryptography.MD5.Create();
} else {
hash = System.Security.Cryptography.SHA512.Create();
}
return hash;
}

Is there a general opinion on which is better style?

Thanks,
Matt
Jun 27 '08 #2
Matt B wrote:
I was just wondering if there is a "best" choice from the following
couple of ways of returning a value from a method:

1)
private HashAlgorithm GetSpecificHashAlgorithm(string hashString){
if (hashString == "MD5") {
return System.Security.Cryptography.MD5.Create();
} else {
return System.Security.Cryptography.SHA512.Create();
}
}

or

2)
private HashAlgorithm GetSpecificHashAlgorithm(string hashString){
HashAlgorithm hash;
if (hashString == "MD5") {
hash = System.Security.Cryptography.MD5.Create();
} else {
hash = System.Security.Cryptography.SHA512.Create();
}
return hash;
}

Is there a general opinion on which is better style?
No, but I'm pretty sure there'll be plenty of people to offer their own.
Single-exit versus multiple-exit is as old as there has been a return statement.

As far as I'm concerned, as long as you keep your functions small (which is
always a good idea), this is a bikeshed problem. http://www.bikeshed.com/

Focusing on style, you haven't even addressed the actual problems with your
functions (even granting that they're just meant to illustrate one issue):

- You don't check if the argument is null. Although these functions are
private, I wouldn't omit a simple null check even for private functions,
since it's cheap and very effective at catching problems early. If you were
not expecting "hashString" to ever be null, you would be surprised to
suddenly be working with a SHA512 hash you never thought you requested.

- You don't check if the argument is "SHA512". Anything that's not MD5 is
SHA512? That's not extensible. A dictionary lookup would be more flexible,
and would not require rewriting the function for new algorithms.

- You are duplicating functionality that's already available in the form of
HashAlgorithm.Create().

Finally, and less importantly, while you're free to name private members in
any way you like, it's probably a better idea to not start them with an
uppercase letter, so that it's immediately that it's not a public member (as
those *should* start with an uppercase letter). That's an opinion, though,
as you're free to name your private members however you like.

--
J.
http://symbolsprose.blogspot.com
Jun 27 '08 #3
On Wed, 28 May 2008 10:57:15 -0700, Matt B <ro**********@gmail.comwrote:
I was just wondering if there is a "best" choice from the following
couple of ways of returning a value from a method:

[...]
Is there a general opinion on which is better style?
I doubt you'll find a consensus. :) My preference is for #2, because it
allows me to keep any
common end-of-method processing all in one place. If I write all my
methods like that from the outset, then if I have to add something like
that later, it's already set up for that.

But even with that preference, I have been known to put a "return"
statement in the middle of a method. :)

Pete
Jun 27 '08 #4


"Matt B" <ro**********@gmail.comwrote in message
news:29**********************************@s21g2000 prm.googlegroups.com...
I was just wondering if there is a "best" choice from the following
couple of ways of returning a value from a method:

1)
private HashAlgorithm GetSpecificHashAlgorithm(string hashString){
if (hashString == "MD5") {
return System.Security.Cryptography.MD5.Create();
} else {
return System.Security.Cryptography.SHA512.Create();
}
}

or

2)
private HashAlgorithm GetSpecificHashAlgorithm(string hashString){
HashAlgorithm hash;
if (hashString == "MD5") {
hash = System.Security.Cryptography.MD5.Create();
} else {
hash = System.Security.Cryptography.SHA512.Create();
}
return hash;
}

Is there a general opinion on which is better style?

Thanks,
Matt
Another way:

private HashAlgorithm GetSpecificHashAlgorithm(string hashString) {
if (hashString == "MD5") {
return System.Security.Cryptography.MD5.Create();
}
return System.Security.Cryptography.SHA512.Create();
}

What I would recommend, which furthers Peter's response, is to create an
enumeration of valid hash algorithm names and pass one of these values to
the method instead of a string. Then you can restrict the parameter values
to those listed in the enumeration.

:)

HTH,
Mythran
Jun 27 '08 #5
Peter Duniho <Np*********@nnowslpianmk.comwrote:
I was just wondering if there is a "best" choice from the following
couple of ways of returning a value from a method:

[...]
Is there a general opinion on which is better style?

I doubt you'll find a consensus. :) My preference is for #2, because
it allows me to keep any common end-of-method processing all in one
place. If I write all my methods like that from the outset, then if I
have to add something like that later, it's already set up for that.
How often do you find yourself having to do that in a way which isn't
easily covered with try/finally? I find it's pretty rare - whereas I've
seen *lots* of code which strictly adheres to "thou shalt have a single
exit point" and is painfully complicated because of it.

Personally I almost always write in a style where as soon as you know
the result of the method and you've done everything you need to, just
return. I find it keeps any one individual flow simpler.

As you say though, there's certainly not going to be consensus :)

--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com
Jun 27 '08 #6
How often do you find yourself having to do that in a way which isn't
easily covered with try/finally? I find it's pretty rare - whereas I've
seen *lots* of code which strictly adheres to "thou shalt have a single
exit point" and is painfully complicated because of it.
Been there
Personally I almost always write in a style where as soon as you know
the result of the method and you've done everything you need to, just
return. I find it keeps any one individual flow simpler.
I totally agree, as soon as you finished your processing you should
end it. It's simple and elegant IMO.
Jun 27 '08 #7
On Wed, 28 May 2008 12:52:22 -0700, Jon Skeet [C# MVP] <sk***@pobox.com>
wrote:
>I doubt you'll find a consensus. :) My preference is for #2, because
it allows me to keep any common end-of-method processing all in one
place. If I write all my methods like that from the outset, then if I
have to add something like that later, it's already set up for that.

How often do you find yourself having to do that in a way which isn't
easily covered with try/finally?
I admit, I'm not entirely sure what you mean. I tend to avoid try/finally
unless there's an actual exception I anticipate occurring. Yes, it can be
used even without an exception, but for me, the presence of a "try"
statement sends a strong signal that an exception is anticipated.

If C# allowed for a "finally" statement/block by itself, I might be more
likely to use that as my approach. But given that I can't use "finally"
with a "try", and the extra level of indentation for the entire method
block in that case, and given the "there's an exception" implication of
"try", that's just not an idiom I'm likely to use just for the sake of
cleanup.

In truth, most methods are simple enough that this question just doesn't
even come up. But inasmuch as it does come up, and inasmuch as sometimes
it comes up outside the context of thrown exceptions, I find reasons to
keep a single return statement.
I find it's pretty rare - whereas I've
seen *lots* of code which strictly adheres to "thou shalt have a single
exit point" and is painfully complicated because of it.
Any convention can certainly be abused. Foolish consistencies, and all
that.
Personally I almost always write in a style where as soon as you know
the result of the method and you've done everything you need to, just
return. I find it keeps any one individual flow simpler.
Different situations call for different techniques. But as a general
rule, if you've got a method where there's a question of "any one
individual flow", I do find scenarios where I find it preferable to try to
keep as close to "just one individual flow" as much as possible.
Obviously conditionals always add different code paths, but I like to keep
those paths as short as possible and return to the main flow of the method
as soon as possible. Often this means consolidating shared code into one
place.

But every method is different. As I said, even I have been known to write
more than one "return" statement in a method. :)

Pete
Jun 27 '08 #8
Peter Duniho <Np*********@nnowslpianmk.comwrote:
I doubt you'll find a consensus. :) My preference is for #2, because
it allows me to keep any common end-of-method processing all in one
place. If I write all my methods like that from the outset, then if I
have to add something like that later, it's already set up for that.
How often do you find yourself having to do that in a way which isn't
easily covered with try/finally?

I admit, I'm not entirely sure what you mean. I tend to avoid try/finally
unless there's an actual exception I anticipate occurring. Yes, it can be
used even without an exception, but for me, the presence of a "try"
statement sends a strong signal that an exception is anticipated.
That would be the case for try/catch, but for try/finally I simply
regard it as "do this whatever happens".
If C# allowed for a "finally" statement/block by itself, I might be more
likely to use that as my approach. But given that I can't use "finally"
with a "try", and the extra level of indentation for the entire method
block in that case, and given the "there's an exception" implication of
"try", that's just not an idiom I'm likely to use just for the sake of
cleanup.
How would a finally block by itself work? You need to give some scope
to it: if I exit <thisblock, do <thiswork.
In truth, most methods are simple enough that this question just doesn't
even come up. But inasmuch as it does come up, and inasmuch as sometimes
it comes up outside the context of thrown exceptions, I find reasons to
keep a single return statement.
I'm sure there are some cases where it's useful. I just find they're
relatively rare :)
I find it's pretty rare - whereas I've
seen *lots* of code which strictly adheres to "thou shalt have a single
exit point" and is painfully complicated because of it.

Any convention can certainly be abused. Foolish consistencies, and all
that.
Indeed.
Personally I almost always write in a style where as soon as you know
the result of the method and you've done everything you need to, just
return. I find it keeps any one individual flow simpler.

Different situations call for different techniques. But as a general
rule, if you've got a method where there's a question of "any one
individual flow", I do find scenarios where I find it preferable to try to
keep as close to "just one individual flow" as much as possible.
Obviously conditionals always add different code paths, but I like to keep
those paths as short as possible and return to the main flow of the method
as soon as possible. Often this means consolidating shared code into one
place.
I find that often there are "early outs" which mean that most of the
method doesn't need to operate at all: if a parameter is null, or an
empty string, or an empty list etc. In those cases, where the result is
known without looking at the complex logic later on, why not get out as
quickly as possible? That's the kind of example I keep seeing (or used
to, anyway) where "single return point" has been painful.

Another classic example is looking for something in a loop. I find this
a nice, obvious way of expressing the logic:

Person FindByName(string name)
{
foreach (Person p in people)
{
if (p.Name==name)
{
return p;
}
}
return null;
}

.... and this less obvious:

Person FindByName(string name)
{
Person found = null;
foreach (Person p in people)
{
if (p.Name==name)
{
found = p;
break;
}
}
return found;
}

Reasons:
1) When we've found the person, what do we instinctively want to do?
Return it. Not break out of the loop so we can later return the person
we've just found.

2) When is null (the default value) relevant? When we've just exhausted
all the possibilities - not before we start looking.

Of course, this may be a case where you'd use the first version anyway,
but it's a good example (IMO) of where multiple returns help.
But every method is different. As I said, even I have been known to write
more than one "return" statement in a method. :)
:)

--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com
Jun 27 '08 #9
Thanks for all the replies guys. While my code samples didn't do the
actual code justice, I learned quite a bit from all your responses. I
actually started out thinking that I should be always following
example #2, but I'm not so sure anymore. In any case, by making me re-
think a few things you all have helped to make me a better programmer
today. Thanks!

Matt
Jun 27 '08 #10
On Wed, 28 May 2008 14:52:14 -0700, Jon Skeet [C# MVP] <sk***@pobox.com>
wrote:
[...]
How would a finally block by itself work? You need to give some scope
to it: if I exit <thisblock, do <thiswork.
Well, I'm no grammarist. :) But, I'd specify it sort of like the "yield"
statement is used. That is, if it's present at all, it has some effect on
how the entire method is interpreted by the compiler. Though, in this
case the rules could be simpler: a "finally" clause by itself is required
to be at the top-most scope within a method and must be at the end. If
those rules are met, a "finally" clause without supporting "try" is legal
and simply means to execute all of the code within the clause before
returning to the caller.

I could equivocate on the second requirement (must be at the end); I like
it from a readability point of view, but inasmuch as the first rule gets
most of the readability one needs, and inasmuch as I could see how someone
else would prefer to declare all their "cleanup" code at the beginning,
along with whatever locals and whatnot might actually need to be cleaned
up, I wouldn't argue strenuously for that rule.

I don't really mind that this syntax doesn't exist. The number of times
that it would be applied would be relatively low IMHO. And I like all of
the alternatives just fine (while I wouldn't use try/finally just for this
purpose myself, if someone else wants to that's fine by me). And of
course the "using" statement already accomplishes much of the sort of
cleanup that would have existed in a different environment (*). But it
could be done, and I think it'd be reasonable readable and useful, at
least in some situations.

(*) I think it goes without saying, but maybe it's worth me pointing out
that when I discuss general code layout conventions like this, I am in
fact speaking relatively generally. Many of my habits or opinions come
from years of C/C++ coding, and there's a lot in C# that dramatically
reduces the frequency with which a particular idiom might be needed. But
IMHO, there's no reason to toss the tool from the toolbox just because it
doesn't get used as much as all the others.
[...]
Another classic example is looking for something in a loop. I find this
a nice, obvious way of expressing the logic:

Person FindByName(string name)
{
foreach (Person p in people)
{
if (p.Name==name)
{
return p;
}
}
return null;
}

[...]
Of course, this may be a case where you'd use the first version anyway,
but it's a good example (IMO) of where multiple returns help.
Yes, I would typically use the first version. Maybe I understated my
willingness to use multiple returns. It's just one of many different
coding patterns I use, depending on the exact circumstance.

Pete
Jun 27 '08 #11
Matt B wrote:
I was just wondering if there is a "best" choice from the following
couple of ways of returning a value from a method:

1)
private HashAlgorithm GetSpecificHashAlgorithm(string hashString){
if (hashString == "MD5") {
return System.Security.Cryptography.MD5.Create();
} else {
return System.Security.Cryptography.SHA512.Create();
}
}

or

2)
private HashAlgorithm GetSpecificHashAlgorithm(string hashString){
HashAlgorithm hash;
if (hashString == "MD5") {
hash = System.Security.Cryptography.MD5.Create();
} else {
hash = System.Security.Cryptography.SHA512.Create();
}
return hash;
}

Is there a general opinion on which is better style?
Ignoring the issues with this specific code then I would go for #1.

I don't there is a problem with multiple returns. It saves the time of
checking the rest of the code when reading.

Arne
Jun 27 '08 #12
Peter Duniho <Np*********@nnowslpianmk.comwrote:

<snip stuff which is mostly a matter of discussion>
By the way, note that it's not true that "this just isn't an issue in
C#". C#'s rules mitigate the situation a lot, but you can still assign a
boolean in an if() statement without an error.
True - but how often do you actually compare with a boolean constant in
the first place? I always write

if (condition)
or
if (!condition)

instead of

if (condition == true)
or
if (condition == false)

anyway. I'd consider the top options to be the idiomatic C# style.
Allow me to amend my claim to "this just isn't an issue in idiomatic
C#" :)
(You do get a warning, but
again...that's something that's been in at least the one C/C++ compiler
I've used for a very long time).
I didn't even know it was a warning in C#, to be honest :)
[...]
I agree that it's still important to be aware of the options available,
but
sometimes they become so rarely useful that it's no longer worth keeping
them as
what you might consider your general coding style. (Generic "you"
here, of course.)

Well, I guess that depends on your definition of "general coding style".
As I said, I don't have a consistent "I always do it one way or the other"
attitude for this specific question. It's more about anticipating
possible future uses, and weighing the relative aesthetics between the
approaches.

Consistency can be a good thing, but it shouldn't be clinged to when there
are more dominant issues. For different people, what defines "more
dominant issues" may vary (and in matters such as these, they practically
always will).
Sure. I guess I'm just saying that in C# trying to keep to a single
return point (when other courses of action are naturally available) is
something which is so rarely explicitly useful that those situations in
which it *is* useful usually point themselves out anyway.

--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com
Jun 27 '08 #13

This thread has been closed and replies have been disabled. Please start a new discussion.

Similar topics

4
by: Robert Ferrell | last post by:
I have a style question. I have a class with a method, m1, which needs a helper function, hf. I can put hf inside m1, or I can make it another method of the class. The only place hf should ever...
6
by: Master of C++ | last post by:
Hello Group, Please take a look at the following (simple) C++ code (at the end of this post) that does operator overloading. It gives me the following compile error: complex.cpp: In function...
7
by: Calan | last post by:
Mike, Your code on the dynamic input checking was excellent and very well explained. (The only thing I had to do was change the test for text input to be "1 > len of text", instead or "0 >...
27
by: Terry Olson | last post by:
I'm trying to build a table on an output page based on text input by the user. And what I am trying to do is create 4 table data boxes on a row, then start a new row on the 5th one. But I can't...
4
by: Nigel Molesworth | last post by:
I've Googled, but can't find what I need, perhaps I asking the wrong question! I want a "FAQ" page on a web site, I hate those pages that scroll you to the answer so and I figured that a good...
8
by: George | last post by:
I need help with the code listed below. See the line below the comment-// *** This displays the error *** I want to be able to have the event handler call the function based on the reference...
6
by: Ashok | last post by:
Hi, I am starting a new project to build a software product using APS.NET 2.0. In past I have used "frameset" and "frame" to build pages. My current requirements I have coded using frameset and...
1
by: TopherB | last post by:
It came to me while sleeping that my question was not being answered (all answered where in the form of changing the question) because the answer was not readily available. So I thought I might ask...
25
by: bonneylake | last post by:
Hey Everyone, Well i am not sure if my question needs to be here or in coldfusion. If i have my question is in the wrong section i am sorry in advance an will move it to the correct section. ...
3
by: vanald04 | last post by:
Tried searching for the right article and saw some similar questions that if I knew more would probably be helpful, but I couldn't get the coding exactly right for my specific situation. So, thanks...
0
by: DolphinDB | last post by:
Tired of spending countless mintues downsampling your data? Look no further! In this article, you’ll learn how to efficiently downsample 6.48 billion high-frequency records to 61 million...
1
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM). In this month's session, we are pleased to welcome back...
0
by: Vimpel783 | last post by:
Hello! Guys, I found this code on the Internet, but I need to modify it a little. It works well, the problem is this: Data is sent from only one cell, in this case B5, but it is necessary that data...
0
by: jfyes | last post by:
As a hardware engineer, after seeing that CEIWEI recently released a new tool for Modbus RTU Over TCP/UDP filtering and monitoring, I actively went to its official website to take a look. It turned...
0
by: ArrayDB | last post by:
The error message I've encountered is; ERROR:root:Error generating model response: exception: access violation writing 0x0000000000005140, which seems to be indicative of an access violation...
1
by: PapaRatzi | last post by:
Hello, I am teaching myself MS Access forms design and Visual Basic. I've created a table to capture a list of Top 30 singles and forms to capture new entries. The final step is a form (unbound)...
1
by: Shællîpôpï 09 | last post by:
If u are using a keypad phone, how do u turn on JavaScript, to access features like WhatsApp, Facebook, Instagram....
0
by: af34tf | last post by:
Hi Guys, I have a domain whose name is BytesLimited.com, and I want to sell it. Does anyone know about platforms that allow me to list my domain in auction for free. Thank you
0
by: Faith0G | last post by:
I am starting a new it consulting business and it's been a while since I setup a new website. Is wordpress still the best web based software for hosting a 5 page website? The webpages will be...

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.