|
Yes, and yes. But if you do that frequently, better to cache the names and values; may I suggest my EnumTransmogrifier[^]?
And in the spirit of the season I won't complain about the multiple return statements.
|
|
|
|
|
Well I've tried to stay out of that.
I think it's a mistake to become obsessive about 'programming rules' in cases where they are of marginal benefit. There is of course a difference between this (the two returns are pretty obvious) and yards of obscure code salted with multiple return statements.
JustAStupidGurl
|
|
|
|
|
justastupidgurl wrote: There is of course a difference between this (the two returns are pretty obvious) and yards of obscure code salted with multiple return statements.
Yes indeed. Definitely easier to take this way.
|
|
|
|
|
You are right. Building of lookup table improves performance.
static Dictionary<string, StopBits> parsedStopBits = new Dictionary<string, StopBits>();
static ClassName()
{
foreach (StopBits sb in Enum.GetValues(typeof(StopBits)))
{
parsedStopBits.Add(sb.ToString(), sb);
}
}
static StopBits stopBitsFromString3(string stopBitsAsString)
{
StopBits result;
if (!parsedStopBits.TryGetValue(stopBitsAsString, out result))
result = StopBits.None;
return result;
}
Difference between performance of this code and Enum.Parse one is 10 times faster. Difference between perfromance of this code and horror one is 100 times faster.
It is not multiple return you should worry about. It's try-catch. If non-enum value name will be passed in the stopBitsFromString2 function, it's performance will be the same as stopBitsFromString's. Knowing when not to use try-catch is priceless.
modified on Monday, December 15, 2008 12:32 PM
|
|
|
|
|
notmasteryet wrote: It is not multiple return you should worry about.
That's about maintainability, not performance.
notmasteryet wrote: Knowing when not to use try-catch is priceless
Have you read this[^] article?
|
|
|
|
|
1. Always repeat obvious things twice.
Enum NumberSize
Bit_8 = 8
Bit_16 = 16
Bit_32 = 32
Bit_64 = 64
Bit_128 = 128
End Enum
2. Hide all comments behind visible portion of screen - your co-workers do not need them.
Public Function CBin_Number(ByVal InputNumber As Double, ByVal InputNumberSize As NumberSize) As String 'converts large numbers into strings 8, 16,32, 64, or 128 characters long
Dim FinalBinNumber As String
3. Show that 128 zeros and ones can be easily fit in 48 bits mantissa.
Dim TempNumber As Double = InputNumber
Dim PlaceCounter As Integer
4. Always remind that computer cannot be trusted even with simple operation of subtraction.
Select Case InputNumberSize
Case NumberSize.Bit_8
PlaceCounter = 7
Case NumberSize.Bit_16
PlaceCounter = 15
Case NumberSize.Bit_32
PlaceCounter = 31
Case NumberSize.Bit_64
PlaceCounter = 63
Case NumberSize.Bit_128
PlaceCounter = 127
End Select
5. Do not listen anybody (even yourself) and choose most interesting way to handle the task.
If (TempNumber - (2 ^ PlaceCounter)) > 0 Then
PlaceCounter = 127
End If
6. Be simple – the String class is the best thing since sliced bread.
Do Until (PlaceCounter < 0)
If (TempNumber - (2 ^ PlaceCounter)) >= 0 Then
FinalBinNumber = FinalBinNumber & "1"
TempNumber = TempNumber - (2 ^ PlaceCounter)
ElseIf (TempNumber - (2 ^ PlaceCounter)) < 0 Then
FinalBinNumber = FinalBinNumber & "0"
End If
PlaceCounter = PlaceCounter - 1
Loop
7. Always add checks for weird cases
If FinalBinNumber = "11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111" Then
FinalBinNumber = "11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111 or Error: Number Too Large"
End If
Return FinalBinNumber
End Function
Public Shared Function ReaderRev(ByVal BinString As String) As String 'converts strings into ASCII character string
8. Show that you are bilingual, e.g. by using QBASIC and VB.NET constructs
Dim tempstring As String = Replace(BinString, " ", "")
Dim tempbinstr As String
Dim TempChar As Char
9. Do not reveal variable types: SByte can be to small, ULong can be to big.
Dim BinValue As Object
Dim counter As Integer = 0
Dim counter2 As Integer = 1
Dim i As Integer, n As Integer
Dim finalstring As String
10. Hide bugs behind complex computation.
Do Until counter = ((Len(tempstring) \ 8) + 1)
counter = counter + 1
tempbinstr = Mid(tempstring, counter2, 8)
For i = 0 To 7
TempChar = Mid(tempbinstr, (i + 1), 1)
If TempChar = "1" Then
BinValue = BinValue + (2 ^ (7 - i))
End If
Next
finalstring = finalstring & Chr(BinValue)
BinValue = 0
counter2 = counter2 + 8
Loop
Return finalstring
End Function
|
|
|
|
|
Hi,
horrible indeed...
But why would someone be 'popular among his colleagues' when programming like this? I'd rather think Hey, this is a guy who is not really good in his profession when I read things like that in someone else's code. And this means: Much to do for colleagues (refactoring, hard to read/understand) and much potential trouble (likelihood of errors is high, maintainability is hard) - so he makes his colleagues lifes harder...
Regards
Thomas
www.thomas-weller.de
Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the Universe trying to produce bigger and better idiots. So far, the Universe is winning. Programmer - an organism that turns coffee into software.
|
|
|
|
|
This may help sarcasm[^]
It does not always pass through translation.
Never underestimate the power of human stupidity
RAH
|
|
|
|
|
#7 is my favorite... brilliant error message. "It's either this 200 digit number, or it's too big"
He said, "Boy I'm just old and lonely,
But thank you for your concern,
Here's wishing you a Happy New Year."
I wished him one back in return.
|
|
|
|
|
What wrong with this
notmasteryet wrote: Enum NumberSize
Bit_8 = 8
Bit_16 = 16
Bit_32 = 32
Bit_64 = 64
Bit_128 = 128
End Enum
I think it is good practice.
Do you have any better practice ?
|
|
|
|
|
Enum NumberSize
EightBits = 8
SixteenBits = 16
ThirtyTwoBits = 32
SixtyFourBits = 64
HundredTwentyEightBits = 128
End Enum
IMHO in this particular case, there is no effective use of the enumeration (see #4). There is no checks on invalid enumeration values either; most common call of the CBin_Number function was performed with argument CType(size, NumberSize) where size is an integer.
It is hard to find best practices for enumerations since it just grouped constants. Usually all enumeration replaced by the Strategy Pattern during refactoring (see http://sourcemaking.com/refactoring/replace-type-code-with-state-strategy[^] or http://www.jeremyjarrell.com/archive/2007/10/28/64.aspx[^]).
Type casting to enumeration datatype does not do validation, so a callie have to validate enumaration value by itself. However, by decalaring enumeration as
Enum NumberSize
Bit_8
Bit_16
Bit_32
Bit_64
Bit_128
End Enum
you may perform range (in)validation InputNumberSize < NumberSize.Bit_8 OrElse InputNumberSize > NumberSize.Bit_16 , it also allows compliler to optimize Select statement code execution.
Do you think change NumberSize to an integer type would introduce more problems?
|
|
|
|
|
Found in legacy code...
Dim startbit As Integer = ((6 * data.i) + (3 * data.j) + 16)
startbit = Math.Ceiling(startbit / 16) * 16
If startbit Mod 16 <> 0 Then Throw New Exception("you don't know how to round")
Always check what computer is doing
|
|
|
|
|
notmasteryet wrote: startbit = Math.Ceiling(startbit / 16) * 16
Typical and terrible VB approach. :/
Greetings - Gajatko
Portable.NET is part of DotGNU, a project to build a complete Free Software replacement for .NET - a system that truly belongs to the developers.
|
|
|
|
|
I have a coding rule - avoid "return" somewhere in the middle of a method, try using local variables to compensate instead. Observing this leads to the following (seen in existing code):
if(flagA)
{
if(flagB)
{
if(flagC)
{
if(PromtUser())
{
DoSomething();
}
else
{
DoOtherThing();
}
}
else
{
DoOtherThing();
}
}
else
{
DoOtherThing();
}
}
else
{
DoOtherThing();
}
How I would have written this is:
if(flagA)
{
if(flagB)
{
if(flagC)
{
if(PromptUser())
{
return;
}
}
}
}
DoOtherThing();
I was wondering how you guys feel about it?
Thanks,
Georgi
|
|
|
|
|
You forgot to execute DoSomething() in your second example. That aside...
I don't necessarily dislike "return in the middle." But I expect early returns to be an error condition (i.e. invalid parameter, resource not obtained, etc) to keep the method from continuing. In your example, the early return seems to be the normal, successful condition and that makes me uncomfortable.
So looking at your code samples... Definitely not the first example. If you ever want to add more logic around DoOtherThing() , you've essentially "cut-and-pasted" your code all over the place (a big no-no).
There are a lot of conditions where DoOtherThing() would be executed. According to your logic, if each process (A, B, and C) passes, ask the user if they want to DoSomething() . If user says "no" or any of the processes fail, then DoOtherThing() .
To make my intentions clear, I would try and write the logic exactly as I would describe it (of course, the variables would be more "English-Like" if I knew their purpose). Maybe something like this:
bool DoSomethingNeeded = false;
if (FlagA && FlagB && FlagC)
{
DoSomthingNeeded = PromptUser();
}
if (DoSomethingNeeded)
{
DoSomething();
}
else
{
DoOtherThing();
}
return;
|
|
|
|
|
That is pretty much what I do or just break up the function into smaller parts if there are too many levels of conditionals.
John
|
|
|
|
|
if ( flagA && flagB && flagC && PromptUser() )
{
DoSomething();
}
else
{
DoOtherThing();
}
return ;
Keep in mind that the && operator is short-circuit.
|
|
|
|
|
|
According those many sites, C and C++ do use short-circuiting. As a well-defined part of the language standard. Which means that it's fine to reply on, as long as you feel it won't impact readability.
Java also seems to behave differently from what you said:
http://www.student.cs.uwaterloo.ca/~cs132/Weekly/W02/SCBooleans.html[^]
And anyway, it's very pointless to put extra boolean operators (&&, ||) in a language without defining them to be short-circuiting. There are | and & already, and if there is no short-circuiting they'd be the same.
|
|
|
|
|
harold aptroot wrote: Java also seems to behave differently from what you said:
I think you are right. I was looking at this section[^] of the Java language specification:
15.7.2 Evaluate Operands before Operation<br />
The Java programming language also guarantees that every operand of an operator (except the conditional operators &&, ||, and ? : ) appears to be fully evaluated before any part of the operation itself is performed.
They do explicitly exempt the logical operators. Let's hope that && is not overloaded -- in C++ at least -- then short circuiting does not apply.
Then I was reading this on C++ Sequence Points[^]. It says that "the left operand of the logical AND operator is completely evaluated and all side effects completed before continuing. There is no guarantee that the right operand of the logical AND operator will be evaluated.
Of course, it doesn't say that is guaranteed not to be evaluated, either.
Then it says this: "The controlling expression in a selection (if or switch) statement. The expression is completely evaluated and all side effects completed before the code dependent on the selection is executed."
So, who knows. My conclusion would be that a C/C++ compiler is indeed supposed to do left-right evaluation and short circuiting.
But I still say it's irrelevant. If I have to hunt that hard to get down into the bowels of how a compiler works to determine how my code will execute, I say it's a bad programming practice.
I'll take these guys word for it:
...The moral is that writing code that depends on order of evaluation is a bad programming practice in any language. Naturally, it is necessary to know what things to avoid, but if you don't know how they are done on various machines, you won't be tempted to take advantage of a particular implementation. -- The C Programming Language[^], Kernighan & Ritchie, p. 49.
Enjoy,
Robert C. Cartaino
|
|
|
|
|
Ah good ol' C and C++ with their lack of guarantees
Now that is a real horror
|
|
|
|
|
From the ANSI C99 spec I have:
"
6.5.13-4
Unlike the bitwise & operator, the && operator guarantees left-to-right evaluation; there is a sequence point after the evaluation of the first operand. If the first operand compares equal to 0, the second operand is not evaluated.
"
Even Ritchie's C manual from 1974 says it's left-to-right. (It appears that B does not have these operators.)
In the K&R book, they're talking about a[i]=i++ , not the && operator, plus that's a very old book.
You may certainly go right ahead and write code any way you like... I do.
|
|
|
|
|
Robert.C.Cartaino wrote: I think C++ has an "undefined order of evaluation".
C and C++ are both strict left to right evaluation order for logical operators, and always use short-circuiting. It allows constructs such as
void* p = GetPointer();
if (p != NULL && p->SomeMethod()
....
p->SomeMethod() will only be called if p is not NULL
|
|
|
|
|
see this[^]
"
&& Operator (C# Reference)
The conditional-AND operator (&& ) performs a logical-AND of its bool operands, but only evaluates its second operand if necessary.
"
"
x && y
if x is false, y is not evaluated (because the result of the AND operation is false no matter what the value of y may be). This is known as "short-circuit" evaluation.
"
Any language that doesn't do it that way is not a "real" programming language.
|
|
|
|
|
Robert.C.Cartaino wrote: Writing code that depends on order of evaluation is bad programming practice
If order of evaluation is not defined by the specification of the language, which is not true for logical operators in C/C++.
According to your statement one should not write something like this:
if( node && node->value )
node->value->Do();
Instead you need to have another if for this simple operation even if the previous example is perfectly legal and clean and the desired behavior is guaranteed by the language specification. Well I guess that the only bad programming practice here is that someone introduces unneeded code because he doesn't trust/know language.
|
|
|
|