|
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.
|
|
|
|
|
Indeed. And should we question the order/precedence of the other operators as well?
Can we trust 3 + 4 * 5 to return 23? or might it return 35 once in a while?
I'm all for adding parentheses for clarity, but I don't get that confused with necessity.
|
|
|
|
|
You are depending on the order of evaluation of your if-statement for your logic to work. Will the PromptUser() operand be evaluated before the entire set of && operations are performed? I think C++ has an "undefined order of evaluation". Who knows. It should be irrelevant. Writing code that depends on order of evaluation is bad programming practice, regardless of language.
Sorry, but not knowing how your programming language works is bad practice (operator priority is very important in every language!). And depending on the order of your evaluations is the usual case in real world programming-logic. (I think you are mixing up here with the | and & operators and something a teacher told you once...).
I think C++ has an "undefined order of evaluation". Who knows
No - and I!!!
|
|
|
|
|
The second certainly looks cleaner; but sometimes you need to do cleanup (dispose objects, free memory, release handles, etc.) before exiting a function / method. If you add code later that needs cleanup, and already coded the second way, you'll have a lot to change... in addition, code that is already out there running stable is more valuable than new code, since you risk introducing bugs by changing from your second example back to the first. So I'd definitely go for the first - good habits breed consistency.
|
|
|
|
|
I've got to say, i don't agree with that rule.
I will aim to return out the function either very early, or at the end. Trivial conditions should be treated as such, and I'd rather they returned at the top, than add a unnecessary level of nested bracers.
I have seen the result of blindly following that practice to it's conclusion - a mountain of if / else statements with embedded switches that was 12 deep in places. I would rather see:
<br />
<br />
if (guardCondition1)<br />
return guard1Default;<br />
<br />
if (guardCondition2)<br />
return guard2Default;<br />
<br />
if (guardCondition3)<br />
return guard3Default;<br />
<br />
<br />
<br />
<br />
return result;<br />
<br />
That said, throwing return statements in without any real eye on program flow can cause just as many problems.
Regards
Tris
-------------------------------
Carrier Bags - 21st Century Tumbleweed.
|
|
|
|