|
Scott Barbour wrote: cannot even begin to imagine that it is even remotely necessary for shipping packages.
Same here.
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
"Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
|
|
|
|
|
I guess the message was "Drive not A not Ready" from crss.exe. The reason for that is simple.
You have in your application somewhere a drive letter selection combo box.
That little snippet should exhibit this problem:
foreach (DriveInfo d in DriveInfo.GetDrives())
{
Console.WriteLine("Root: {0}", d.RootDirectory);
}
All you have to do is to check the DriveType if it is removeable and skip it altogether. If you do not access it then you wont have any problems with drive not ready messages.
Yours,
Alois Kraus
|
|
|
|
|
I found this in some legacy code at a place where I worked:
public class MyClass
{
private FooEventArgs Foo_Event;
public void MyMethod()
{
Foo_Event = new FooEventArgs( … );
.
.
.
}
}
Foo_Event is used only in MyMethod, but it is declared as an instance variable of the class. At least they did not make it public  .
Also I really don't like having "_Event" appended to the variable name. It isn't an event it is event arguments.
Of course, this pattern was repeated for several variables.
Bill W
|
|
|
|
|
Just refactor the code. 
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
"Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
|
|
|
|
|
Yep, that is exactly what I did
I just find this kind of code irksome.
Bill W
|
|
|
|
|
Hopefully that particular coder isn't somewheres else coding mission critical stuff...
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
"Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
|
|
|
|
|
I just know they are no longer working at the same place. But, the code does work so no one, other than fellow programmers, would even be aware that there is something wrong with the code. This is one of the reasons that I believe in peer code review, not to critize, but to give folk a chance to learn better ways of doing things.
Bill W
|
|
|
|
|
CIDev wrote: This is one of the reasons that I believe in peer code review, not to critize, but to give folk a chance to learn better ways of doing things.
I totally agree. A peer review of the code you posted could have perhaps helped the coder improve. Of course that depends if they are open to constructive criticism.
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
"Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
|
|
|
|
|
I agree with the code review and that one hopes the programmer is open for constructive criticism. However, the one giving the advice should also be open to rebuttal. I've been in situations where I give an answer, in a nice, logical tone and got blasted for going against the criticism. I wasn't trying to be rude, but explaining why I did what I did. Of course, the guy giving the criticism was a hot head to begin with, which may not have helped. 
But yes, code reviews are very helpful and a good learning environment as long as all parties are open-minded to some degree.
|
|
|
|
|
sa_runner wrote: the one giving the advice should also be open to rebuttal
I agree.
sa_runner wrote: the guy giving the criticism was a hot head to begin with
So like Donald Duck?
sa_runner wrote: as long as all parties are open-minded to some degree
That is indeed a necessity for a code review.
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
"Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
|
|
|
|
|
Working on an old vb6 project I found this treasure ...
Function conv8(number) As String
If number = "" Then
number = 0
End If
conv8 = Hex(Numero)
Select Case Len(conv8)
Case 1
conv8 = 0 & 0 & 0 & 0 & 0 & 0 & 0 & conv8
Case 2
conv8 = 0 & 0 & 0 & 0 & 0 & 0 & conv8
Case 3
conv8 = 0 & 0 & 0 & 0 & 0 & conv8
Case 4
conv8 = 0 & 0 & 0 & 0 & conv8
Case 5
conv8 = 0 & 0 & 0 & conv8
Case 6
conv8 = 0 & 0 & conv8
Case 7
conv8 = 0 & conv8
End Select
End Function
Obviously they wrote also a Conv1, Conv2, Conv4 ... and thank god there was no need for a Conv64 !!!!
Don't write code: generate it!
|
|
|
|
|
Funny.
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.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
[My articles]
|
|
|
|
|
A better title would be "Left-padding Made Difficult"
|
|
|
|
|
This bad on so many levels.
Let's look at:
0 & 0 & 0 & 0 & 0 & 0 & 0 & conv8
Remeber kids, conv8 is a Variant so it will be type checked even though it is /probably/ a String.
Every 0 is an Integer that will be cast to a string. Then EACH AND EVERY & creates a new string, which gets thrown away with the next &.
This is so bad, I love it!
Reminds me of a way back when fixed length message that we had in a system if days gone by, that was built up using a this & that & the & other type assignment. The message was regularly being buit up and broken down.
The breakdown would be
this = Left$(message,4)
message = Mid$(message. 5)
that = ...
Changed it to use a function that built a fixed length string that then got updated. Boom, super fast (by VB standards)
Similarly the break up was changed to only extract what was needed and used Mid$() exclusively.
It still holds true today. String manipulation normally requires memory allocation. Work out what you need FIRST and reduce the re-allocs every 3 steps of the friggin way.
Slightly harder to code, but when performance maters it is important.
Panic, Chaos, Destruction.
My work here is done.
|
|
|
|
|
williamnw wrote: Every 0 is an Integer that will be cast to a string. Then EACH AND EVERY & creates a new string, which gets thrown away with the next &.
For all its faults, the VB6 compiler is a little better than that - it is capable of concatenating string literals at compile time.
I remember maintaining a report-writing program that abused VB's string handling similarly. It created an RTF report, but generally appended characters pretty much 1 to 5 at a time, and the eventual reports ran up to about 14MB as RTF, so you can imagine how many string copy's resulted there.
I ported the core of the engine to C++, resulting in a speedup well over one thousand fold!
|
|
|
|
|
Once, in the long-ago murky past, before learning the wonders of Int32.ToString("Dx"), and still being relatively new, I wrote something like this
public string Pad(int val, int places)
{
int realplaces=0;
if (val < 10) // notice that this includes all negatives, so FAIL
realplaces=1;
else if (val < 100)
realplaces=2;
else if (val < 1000)
realplaces=3;
...
if (realplaces >= places)
return val.ToString();
return new string(' ', places - realplaces) + val.ToString();
}
|
|
|
|
|
Well, you could also learn the wonders of Math.Log10... 
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.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
[My articles]
|
|
|
|
|
string s = number.ToString();
if (s.Length >= placesWanted)
return s;
return new string(' ', placesWanted - s.Length) + s;
Because I still dislike the failure of 1.5 + 1.5 == 3
|
|
|
|
|
What has the above to do with Math.Log10?
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.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
[My articles]
|
|
|
|
|
I prefer to avoid doubles when possible, even if it's a stupid idea. And why wastefully calculate the log when you already have the actual length of the number right in your hands?
|
|
|
|
|
Stefano Basili wrote: thank god there was no need for a Conv64 !!!!
No kidding. It would be ugly, but doable with just loads of copying/pasting 
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
"Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
|
|
|
|
|
Hey! Did you post this code with the author's consent? Gosh, he must've had a hard time on this. Haha
|
|
|
|
|
Sure ... and the original code was even worse than that!
Don't write code: generate it!
|
|
|
|
|
try { oXMLRecord.appendChild(oXMLBlock.selectSingleNode("block_name").cloneNode(true)); } catch(e) {};
try { oXMLRecord.appendChild(oXMLBlock.selectSingleNode("block_type").cloneNode(true)); } catch(e) {};
try { oXMLRecord.appendChild(oXMLBlock.selectSingleNode("title").cloneNode(true)); } catch(e) {};
try { oXMLRecord.appendChild(oXMLBlock.selectSingleNode("list_id").cloneNode(true)); } catch(e) {};
try { oXMLRecord.appendChild(oXMLBlock.selectSingleNode("format_number").cloneNode(true));} catch(e) {};
try { oXMLRecord.appendChild(oXMLBlock.selectSingleNode("format_force").cloneNode(true));} catch(e) {};
try { oXMLRecord.appendChild(oXMLBlock.selectSingleNode("visible").cloneNode(true));} catch(e) {};
try { oXMLRecord.appendChild(oXMLBlock.selectSingleNode("titre").cloneNode(true));} catch(e) {};
try { oXMLRecord.appendChild(oXMLBlock.selectSingleNode("bloc_width").cloneNode(true));} catch(e) {};
try { oXMLRecord.appendChild(oXMLBlock.selectSingleNode("bloc_sens").cloneNode(true));} catch(e) {};
try { oXMLRecord.appendChild(oXMLBlock.selectSingleNode("bloc_data_type").cloneNode(true));} catch(e) {};
try { oXMLRecord.appendChild(oXMLBlock.selectSingleNode("locked").cloneNode(true));} catch(e) {};
try { oXMLRecord.appendChild(oXMLBlock.selectSingleNode("display_type").cloneNode(true));} catch(e) {};
Why have methods when you can just copy-paste the code you need onto each line? And why bother checking such things as if a node exists before trying to do anything with it when you can just catch any exceptions?
This is from a 4165 line javascript file of consistently awful code. The block above is a small sample but there were perhaps three times as many lines just like those where only the name of the child node changes. Another function that could have been somewhat improved had the programmer taken a minute to use an array and a loop rather than his copy-paste approach to everything:
function displayButtonsBlocExistForCell(sCell)
{
if(sCell=="A1")
{
disableAllButtons()
}
else
{
if(sCell.substring(0,1)=="A"||isRow1(sCell))
{
disableControlButton("btnBodyColumn");
disableControlButton("btnBodyLine");
disableControlButton("btnBodyText");
enableControlButton("btnBodyModify");
disableControlButton("tdAddNewDocLink1");
disableControlButton("tdAddNewDocLink2");
disableControlButton("tdAddComment");
disableControlButton("tdDelComment");
disableControlButton("btnEditDelete");
if(sCell.substring(0,1)=="A")
{
enableControlButton("btnEditInsertRows");
enableControlButton("btnEditDeleteRows");
disableControlButton("btnEditInsertCols");
disableControlButton("btnEditDeleteCols");
}
else
{
disableControlButton("btnEditInsertRows");
disableControlButton("btnEditDeleteRows");
enableControlButton("btnEditInsertCols");
enableControlButton("btnEditDeleteCols");
};
};
};
};
Also notice the additional semi-colons..! Sure, it only adds an empty statement and you can write ;;;; anywhere in between statements or declarations in JS without changing anything, but much like the if (myVar == true) bunch it doesn't exactly make you look like you know what you're doing or care much about doing it properly.
(In case anyone don't get the thing with the == true, adding == true to any boolean expression results in another boolean expression that is necessarily equivalent to the original boolean expression. Another, even stupider variant is the ternary myVar? true : false... Sigh.
|
|
|
|
|
Nice, very nice...  My apologies if this is the code that you are currently trying to deal with (and it does sound like that is the case).
As for me, I'm feeling kind of happy right now because I am in the midst of starting up a new project that will be all new development so I won't have to be dealing with this kind of nonsense. If I'm lucky, development will be controlled enough that none of this will crop in to the code base either (I'm probably not that lucky).
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
