|
Whilst checking for warnings is important, it can (as you point out) become so overriding that checking for the code working becomes insignificant.
The attitude becomes "Oh dear, variable 'x' is uninitialised, I'll just change it from int x; to int x = 0;. Phew! Job done." without even stopping to consider what x is being used for. Far better to have something like
int x; and just accept that there will be a warning. I have seen code that looks like
int x = 0;
|
|
|
|
|
Firstly, your assertion is slightly wrong. Uninitialised variables generate a warning if the code path allows them to be accessed before being set.
As an example the following will cause a build warning on the last line:
Integer x;
if (something()) {
x = 3;
}
int y = x * 4;
Secondly, without a good reason, variables used within a loop should be defined in the loop.
Panic, Chaos, Destruction. My work here is done.
Drink. Get drunk. Fall over - P O'H
OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre
I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer
Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
|
|
|
|
|
Nagy Vilmos wrote: variables used within a loop should be defined in the loop
Surely you meant inside condition, not loop? (BTW, I know your name is not Shirley  )
modified 19-Nov-18 21:01pm.
|
|
|
|
|
I've not seen this with locals too often (the code walker is quite good at checking them), but it is possible to set up conditions such that the code really, honestly, has to assign to x before reading it, but the compiler can't work it out. I think you can do it with
int x;
if(a) x = 5;
if(a){
return x;
}
What seems to be more common is the 'member X is declared but never used', if you're implementing an interface but not actually using it. For example let's say we have a collection of objects that we're listening to notifications on, but some of the classes don't actually need to notify:
class DontBother : INotifyPropertyChanged {
public event PropertyChangedEventHandler PropertyChanged;
}
class MainClass {
List<INotifyPropertyChanged> sources;
void SomeMethod(){
sources.Add(new DontBother());
}
}
The declaration in DontBother will, I think, give you a compiler warning. Maybe it's not exactly this scenario, I forget, but it's similar. Sometimes the only way to get a clean compile is to use #pragma warning disable  .
|
|
|
|
|
There could be a cleaner, pedantic-like way:
#pragma warning(suppress: 4700)
int x;
|
|
|
|
|
In VS there is a very handy little option: Project Properties, Build tab, "Treat warnings as errors" - switch it to "All".
If you don't fix what it complains about, you can't run it.
Modern compilers complain for good reason - it's not like the old days - and if they say "this is not initialized" then it isn't - and if it doesn't give you a problem now, it will later on when you have forgotten that you are ignoring the warning. That's when it gets expensive to fix a problem...because the code that causes it is is a different module, in a different DLL!
Ignoring warnings is lazy stupidity, and you can tell him that from me!
Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water
|
|
|
|
|
Actually Its PHP. It has the capabilities to overlook warning and errors but the worse case occurs in case of syntax errors. If there is a syntax error the entire page goes blank. Than I had read carefully to get the syntax error 
|
|
|
|
|
Take a look at the algorithm
function Process()
{
if(error!="")
{
// Some HTML code over 1000 lines with javascript
}
else if(update!="")
{
// Same like above :o with same f-king javascript
}
else
{
// Same 1000 of lines with same javascript.
}
}
now my job is to make some change, for new requirements. the file is over 4000 lines. And reading over hour I actually failed to figure out how actually it works. Authority got that reason, I am not good enough. I agreed with them.
Oh the reason to change the code was "Performance and incomplete verification"
|
|
|
|
|
Be very careful with code like this. You think it's the same 1000 lines but you may find one line has been changed. Seen this rubbish many times before, if you refractor to make sense of it, you'll miss that one line.
"You get that on the big jobs."
|
|
|
|
|
Yep I was worried about that too and I know the differences. And thinking of future I redesigned the entire page. Now even I need to change its only one function.
|
|
|
|
|
As Rob says, make sure you are entirely sure of any changes between blocks and that they are maintained.
I have encountered this many times while translating old classic asp applications to newer dotNet. I found that by splitting each of the cases into a separate text file and then running diffs on them tends to make the job much easier. The good part is that most devs who have done this used copy-paste and then made their modifications, so there is some hope for them yet...
I wasn't, now I am, then I won't be anymore.
|
|
|
|
|
I was going to suggest exactly this little trick. It can really help with refactoring old crap.
|
|
|
|
|
Following from rob, take the three code blocks and diff them.
Panic, Chaos, Destruction. My work here is done.
Drink. Get drunk. Fall over - P O'H
OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre
I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer
Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
|
|
|
|
|
Or even better, throw them away. Rewriting from scratch can be less work in such cases.
And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"
And I smiled and was happy
And it came worse.
|
|
|
|
|
|
private string GetTime(string strShiftCode, string strTime)
{
string strReturn = "";
switch (strTime)
{
case "01":
strReturn = "AM";
break;
case "02":
strReturn = "AM";
break;
case "03":
strReturn = "AM";
break;
case "04":
strReturn = "AM";
break;
case "05":
strReturn = "AM";
break;
case "06":
strReturn = "AM";
break;
case "07":
strReturn = "AM";
break;
case "08":
strReturn = "AM";
break;
case "09":
strReturn = "AM";
break;
case "10":
strReturn = "AM";
break;
case "11":
strReturn = "AM";
break;
case "12":
strReturn = "PM";
break;
case "13":
strReturn = "PM";
break;
case "14":
strReturn = "PM";
break;
case "15":
strReturn = "PM";
break;
case "16":
strReturn = "PM";
break;
case "17":
strReturn = "PM";
break;
case "18":
strReturn = "PM";
break;
case "19":
strReturn = "PM";
break;
case "20":
strReturn = "PM";
break;
case "21":
strReturn = "PM";
break;
case "22":
strReturn = "PM";
break;
case "23":
strReturn = "PM";
break;
case "24":
strReturn = "PM";
break;
}
return strReturn;
}
Found this one on one of the project assigned to me today.... It really is about quality control.. 
One variable passed is never been used in function and do I really need a function ?
My solution :
string strReturn = Int32.Parse(strTime.Trim())<12 ? "AM" : "PM";
Zen and the art of software maintenance : rm -rf *
Math is like love : a simple idea but it can get complicated.
|
|
|
|
|
In any case I would also look at all places where the function is called. There this unused parameter must be passed. It's better when you find out what this 'ShiftCode' may have been intended for.
And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"
And I smiled and was happy
And it came worse.
|
|
|
|
|
It's also wrong : "24" is "AM"
Cheers
If you can read this, you don't have Papyrus installed
|
|
|
|
|
And that again may also be wrong. Are the hours not enumerated 0 - 23, totally VB unfriendly?
And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"
And I smiled and was happy
And it came worse.
|
|
|
|
|
The application is used to monitor Robots in factory so may be hour 24 is AM for them... I don't speak Robolang yet ... 
Zen and the art of software maintenance : rm -rf *
Math is like love : a simple idea but it can get complicated.
|
|
|
|
|
Yes, you and I (and probalby most people) would calculate that way.
In the past I have once or twice seen the time expressed as 24:xx.
Anyhoo, if you inspect the code you will see 11 AM's and 13 PM's.
Cheers
If you can read this, you don't have Papyrus installed
|
|
|
|
|
Time and dates can be hell, even if you have a decent framework at your disposal. If you do such things on the fly: Good luck.
And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"
And I smiled and was happy
And it came worse.
|
|
|
|
|
You never get "24" it's "00"
|
|
|
|
|
That depends entirely on the input, we're not dealing with a real time format here.
As said by others here, thse method is wrong in many respects.
My comment denoted that there are 11 "AM" entries and 13 "PM" entries.
So if "24" is input, the output should logically be "AM", not "PM".
Cheers
If you can read this, you don't have Papyrus installed
|
|
|
|
|
ISO 8601 allows 24:00 as the end of the day, so it would be PM; AM starts at 00:00 (not that ISO 8601 recognizes AM/PM of course).
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
