|
That is the first thing I thought of when I saw this. The thought of converting floating point to integers using casts keeps me awake at night.
I am actually currently working on a project at work that has a few of these. I just started here at the time I noticed them. For fun, I thought it would be neat to try compiling this project (which was developed in VC++ 6.0) with VC 2005, and VC 2005 choked on these instances. Good job to the compiler team on that one. It also choked on a few things that I thought VC++ 6.0 should have, such as assigning an int literal to a CString. A few more bonus points to the team there, although 6.0 definitely should have caught that.
Of the pow(int,int)s, at least one could've used the x *= 2 form, as it was in a loop, and the others the shift form. In the end, I decided not to touch any of them, because for all I know the truncation does mess things up but somebody spent a bunch of hours putting in "fixes" to work around this. The routines in question don't seem to have any bugs for now, but since a lot of the other ones do, I decided to focus my energy on those.
|
|
|
|
|
Gary Wheeler wrote: I replaced it with the following expression:
And in the process introduced a subtle bug 
|
|
|
|
|
Good catch!
I assume you are referring to the different results when having i with values smaller than 0 and bigger than 31...
Robert
|
|
|
|
|
Just smaller than 0, but as he is casting to int 31 or above could be a problem too, not sure if C (which I assume this is) will create a 'long' or 'long long' or whatever they use from the shift. But the result of usage of negative numbers are clear undefined.
|
|
|
|
|
Actually not; see my reply above.
Software Zen: delete this;
|
|
|
|
|
The code got optimized for readability for the 'junior' programmers.
And it has the potential to get easily chanced to (int)pow(3,i).
I say this to defend your senior... 
Greetings from Germany
|
|
|
|
|
KarstenK wrote: defend your senior...
Actually, I'm senior to the guy that wrote this. I just thought that the guy should have picked a better way to achieve the desired affect.
Software Zen: delete this;
|
|
|
|
|
<font color="Blue">int</font> pow<font color="DarkBlue">(</font><font color="Blue">int</font> i<font color="DarkBlue">,</font> <font color="Blue">int</font> j<font color="DarkBlue">)</font>
<font color="DarkBlue">{</font>
<font color="Blue">switch</font> <font color="DarkBlue">(</font>j<font color="DarkBlue">)</font>
<font color="Blue">case</font> <font color="Red">0</font><font color="DarkBlue">:</font> <font color="Blue">return</font> <font color="Red">1</font><font color="DarkBlue">;</font>
<font color="Blue">case</font> <font color="Red">1</font><font color="DarkBlue">:</font> <font color="Blue">return</font> i<font color="DarkBlue">;</font>
<font color="Blue">case</font> <font color="Red">2</font><font color="DarkBlue">:</font> <font color="Blue">return</font> i <font color="DarkBlue">*</font> i<font color="DarkBlue">;</font>
<font color="Blue">case</font> <font color="Red">3</font><font color="DarkBlue">:</font> <font color="Blue">return</font> i <font color="DarkBlue">*</font> i <font color="DarkBlue">*</font> i<font color="DarkBlue">;</font>
<font color="Blue">case</font> <font color="Red">4</font><font color="DarkBlue">:</font> <font color="Blue">return</font> i <font color="DarkBlue">*</font> i <font color="DarkBlue">*</font> i <font color="DarkBlue">*</font> i<font color="DarkBlue">;</font>
<font color="DarkBlue">.</font><font color="DarkBlue">.</font><font color="DarkBlue">.</font>
<font color="DarkBlue">}</font> 
|
|
|
|
|
Nope.
You forgot break; statement. Oh pardon, you're senior! 
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
[my articles]
|
|
|
|
|
|
<br />
int pow(int i, int j)<br />
{<br />
switch (j) {<br />
default: return 0;<br />
case 0: return 1;<br />
case 32: i *= i;<br />
case 31: i *= i;<br />
case 30: i *= i;<br />
...<br />
case 1: return i;<br />
}<br />
}<br />
....
ewwwww
Tim Smith
I'm going to patent thought. I have yet to see any prior art.
|
|
|
|
|
|
(stack madness variant)
int imul(int i, int j)
{
if (j==0 ) return 0;
else return imul(i,j-1) + i;
}
int ipow(int i, int j)
{
if (j==0) return 1;
else return imul(ipow(i,j-1) , i);
}
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
[my articles]
|
|
|
|
|
Gary Wheeler wrote: This was in code written by a senior developer
When seniority approaches retirement...
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
[my articles]
|
|
|
|
|
Do you work for the same company as me? I've seen someone write code just like that -- to create a bitmask.
|
|
|
|
|
But... but... but... this ISN'T VB...
Is it possible to have bad code written by a senior developer in a non-VB language???
|
|
|
|
|
We senior developers are very resourceful! 
I want to die like my Grandfather.
Peaceful, Sleeping.
Not screaming like his passengers.
|
|
|
|
|
Gary Wheeler wrote: (1 << i)
Moral of the story. Shift happens.
|
|
|
|
|
<DeepMysteriousVoice>
You have become known to us. Your capacity for puns is disturbing...
</DeepMysteriousVoice>
Software Zen: delete this;
|
|
|
|
|
Gary Wheeler wrote: <deepmysteriousvoice>
You have become known to us. Your capacity for puns is disturbing...
A curse upun you.
|
|
|
|
|
Quoting Peter Lorre in The Raven[^]:
"Ah! You defend yourself, you coward!"
Software Zen: delete this;
|
|
|
|
|
It's a documentation rather than a coding horror, but to good to let it pass.
Either I'm seriously missing something, or MSDN has gone really bonkers.
msdn:DllMain[^] writes:
There are serious limits on what you can do in a DLL entry point. To provide more complex initialization, create an initialization routine for the DLL. You can require applications to call the initialization routine before calling any other routines in the DLL.
So far so good, so true.
But now, what does MSDN recommend to make that initialization automatic?
Alternatively, the initialization routine can create a file with an ACL that restricts access, and each routine in the DLL would call the initialization routine if the file does not exist.
If complex per-process initialization is required, use a "private" global flag. If cross-process initialization is required (that's what would be closest to creating a file...), use a flag in a shared data segment.
modified on Tuesday, December 25, 2007 4:00:09 PM
|
|
|
|
|
 I am not sure about that.
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
|
|
|
|
|
I agree whole-heartedly with their solution.
peterchen wrote: If complex per-process initialization is required, use a "private" global flag
...You mention "complex" and then "private global flag" in the same sentence??
To me..."complex" would imply...well.....complex.
And "private global flag" would imply....well....an improper implementation for a "complex" solution
A "private global flag" (regardless of the fact that declaration of anything with global scope is incorrect) would create an incredibly fragile, convoluted solution. Creating an access list (along with the other MSDN suggestions) would be extensible and compartmentalized.
"I need build Skynet. Plz send code"
modified on Thursday, January 03, 2008 10:04:15 AM
|
|
|
|
|
the gist of it:
flag=bad;
"I need build Skynet. Plz send code"
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
