For posterity.
-
Me: Module X does not compile after your change because of variable redeclaration in line Z:
int a = 0; int a = 0;
Person responsible for the change (job title: software engineer): Okay, fixed it:
- int a = 0; - int a = 0; ============ + int a = 0; + //int a = 0;
I'm not sure what is more troubling: The 'fix', or how this kind of bug ended up in our integration branch. It effectively means that they (our headquarters - we are an offshore dev branch) are not only skipping tests, but don't even check if their code compiles.
-
@eskel bad merge?
-
@groo Nope. He (She?) made this literal change (p4diff reports it as 2 line change, but only the second one was changed) in their dev branch and then pushed it into the integration branch. Looks like a copy-paste-from-unversioned-stash gone wrong to me, based on previous experience with those guys.
-
I have a non-developer coworker who knows just enough JavaScript to be dangerous in our CMS. She also has a fear of deleting code: she only ever comments it out, no matter how trivial it was, or if there are more commented out lines than active lines. (Eventually there always will be.)
If you have version control, just delete it. You can always get it from an older commit. If it's code like this, consider a career change.
-
@error The joke is on me - I ended up as a dev because I made a career change. Funny thing is I'm supposed to be the noob in this long-distance relationship.
-
@eskel Don't you know you have to just comment out dead code instead of reusing it, in case you need to put it back someday? It's not as if you have some magic way of tracking changes to source code.
-
@FrostCat Yeah, it's sooo useful to have this second declaration right there where I need it, just in case!
The most commented source file I've seen here has some 80% of it's 2k lines commented out and has been unchanged for the past two years.
-
@eskel Dunno why did you trust one of that declarations to be correct? In such a bad code, you should have commented both them and write a new one from scratch.
-
@groo better make sure that it works and is declared properly. Clearly this is the most foolproof method.
int a; try{ int a = 0; ..... catch(Exception e){ a = 0; ..... }
-
@eskel said in For posterity.:
Yeah, it's sooo useful to have this second declaration right there where I need it, just in case!
My favorite comments are the ones that explain the code beneath them.
// declare variable foo var foo; // assign 5 to foo foo = 5;
(This was something I actually had to do in my computer science classes, because every statement had to be commented no matter how mundane.)
Edit: even better when the code gets updated and not the comment, so now the comments are lying to you.
// assign 5 to foo foo = 4;
-
@error said in For posterity.:
(This was something I actually had to do in my computer science classes, because every statement had to be commented no matter how mundane.)
Yeah. It was annoying as all get out, but sometimes I find myself doing it on accident anyways...
-
@Tsaukpaetra My general guideline for comments is, "tell me why, not what." And an ounce of good names is worth a pound of comments.
-
@error said in For posterity.:
And an ounce of good names
theTable
,value
,p
,DataTypeType
isn't clear enough for you? :POh well, at least I'm not as bad as @SpectateSwamp (apparently)...
Is it bad that I just saw the
if (value.Substring(10, 1) == "T" && value.Substring(13, 1) == ":")
line and wondered if using RegEx would have been more reliable?Help!
-
@Tsaukpaetra You've already descended down the reflection rabbithole. Your soul cannot be saved.
-
@Tsaukpaetra I can't say that an RE wouldn't be better there.
-
@error said in For posterity.:
@Tsaukpaetra You've already descended down the reflection rabbithole. Your soul cannot be saved.
To be fair, I think Reflection was probably justified here, as the helper is expected to generate a sheet dynamically based on whatever List<T> object it's given:
private void GenerateWorksheet<T>(WorksheetCollection sheets, List<T> theTable, string HeaderCode, bool PrintLandscape)
@pydsigner said in For posterity.:
@Tsaukpaetra I can't say that an RE wouldn't be better there.
I think this was before I could reliably copy-paste a regex into an interpreter to make sure I was doing it right. Might mark that for improvement...
-
@Tsaukpaetra said in For posterity.:
//Attempt to detect datetime stored as a String if (value.Length == 19) //Example: 2015-06-30T00:00:00 { if (value.Substring(10, 1) == "T" && value.Substring(13, 1) == ":") { ...
As much as I hate to be the guy who says "you know, you should probably use a regex for that..."
...I kinda like:
/^(?=....-..-..T..:..:..)(.?\d){14}$/
-
@Tsaukpaetra why wouldn't you just try to convert to datetime, handle the error if it isn't, and see if it's valid that way? Then you don't have to worry about stupid inputs like "2016-09-31T03:15:22" breaking things on you.
-
@groo said in For posterity.:
@eskel Dunno why did you trust one of that declarations to be correct? In such a bad code, you should have commented both them and write a new one from scratch.
It was their change, not mine. This reminds me of debugging latin 'c' chars randomly mixed into our cyrillic data set. Seems trivial, but can make you tear your beard out in frustration when you don't know what you're dealing with.
-
@darkmatter said in For posterity.:
@Tsaukpaetra why wouldn't you just try to convert to datetime, handle the error if it isn't, and see if it's valid that way? Then you don't have to worry about stupid inputs like "2016-09-31T03:15:22" breaking things on you.
Because I have a dream that in some time in the future, a future TDWTFer will have inherited this codebase, wag their head in wonder, and submit this, whereupon old things will become new and my name will finally be in a bona fide TheDailyWTF article!
...
I have weird dreams sometimes. Better sprinkle my name all over the codebase...
-
@eskel In my dim and distant past I recall being told that under certain circumstances such declarations needed to be declared twice. I do not recall the language being used, but based on my language "history" it could have been C. It may have been related to recasting the type of variable, or something to do with not declaring the type prior to use. At the time, my response was along the lines of "Uhhh, OK".
INB4: I am not defending this philosophy, or even admitting that is something I have done - or do. I just think that the "old" programmer had valid justifications for this and was a habit he had gotten into. I was just commenting in case anybody had any similar "memories" - we are talking a long time ago 20+ years at least.
-
@loose said in For posterity.:
but based on my language "history" it could have been C
Not if it's in the same scope. That would be a syntax error.
-
@dcon said in For posterity.:
@loose said in For posterity.:
but based on my language "history" it could have been C
Not if it's in the same scope. That would be a syntax error.
Which hints as the WTFery of their coding style, no?
-
-
@Lorne-Kates said in For posterity.:
@Tsaukpaetra said in For posterity.:
a bona fide TheDailyWTF article!
Article Title: T Time
Legit!
-
@Tsaukpaetra I was expecting to hate it, but I actually found these comments pretty helpful for understanding the code.
Just... Fix that fucking identation, will you? Ugh.
-
@cartman82 said in For posterity.:
@Tsaukpaetra I was expecting to hate it, but I actually found these comments pretty helpful for understanding the code.
Just... Fix that fucking identation, will you? Ugh.
Sorry, every time I ask VS to "fix" it, it does that, so I stopped fighting it.
-
@error said in For posterity.:
I have a non-developer coworker who knows just enough JavaScript to be dangerous in our CMS. She also has a fear of deleting code: she only ever comments it out, no matter how trivial it was, or if there are more commented out lines than active lines. (Eventually there always will be.)
If you have version control, just delete it. You can always get it from an older commit. If it's code like this, consider a career change.
But... but the LOC drops that way. :(
-
@Tsaukpaetra said in For posterity.:
@dcon said in For posterity.:
@loose said in For posterity.:
but based on my language "history" it could have been C
Not if it's in the same scope. That would be a syntax error.
Which hints as the WTFery of their coding style, no?
Or simple removal of a condition check, and the braces surrounding it in one go, without test compiling the code first before check-in because it's too trival.
(Although judging by the distance of declaration, this is not likely to be the case)
-
To clear up a bit: The first declaration was in the file before the change.The second one was added and was the only modification in that particular edit of the file.
@dcon said in For posterity.:
That would be a syntax error.
This is the point. They didn't even check if it compiled and happily pushed the change to integration.
@loose said:
Are you talking about forward declaration?
-
@cartman82 said in For posterity.:
@Tsaukpaetra I was expecting to hate it, but I actually found these comments pretty helpful for understanding the code.
IMO this commeting style has some didactical value, as it lets the learner state their intent outside of code. Makes it obvious whether they are:
- trying to do the right thing, but fail to write correct code,
- trying to do the wrong thing,
- unable to explain in english what they want to do (usually means big problems, and probably a mistake during recruitment).
In cases of 3. I tried to default to my mother tongue a few times, but it turned out that language proficiency was not a factor.
-
@error said in For posterity.:
because every statement had to be commented no matter how mundane
Did they require the comments to be related to the code, or were you allowed to split up the method documentation into one-word comments and put them before each line?
-
@anotherusername said in For posterity.:
(.?\d){14}
While this does match 14 digits after a decimal point, it also matches like a trillion other things.
-
@eskel I guess it would have been something like that, but it is an old memory. I have been dabbling with programming since I was 17 (which is getting on for 40 years ago). One of the earliest computers I programmed was nearly as old as me (at the time), had no memory to speak of and accepted its programmed by punched tape. The possibility that line interpretation was used may have responsible.
That aside, in this day and age, anybody that adds code and submits it without checking it should have a special place in Hell reserved for them. Then they should be shot on sight to ensure they go there.
-
@error said in For posterity.:
@eskel said in For posterity.:
Yeah, it's sooo useful to have this second declaration right there where I need it, just in case!
My favorite comments are the ones that explain the code beneath them.
// declare variable foo var foo; // assign 5 to foo foo = 5;
(This was something I actually had to do in my computer science classes, because every statement had to be commented no matter how mundane.)
Edit: even better when the code gets updated and not the comment, so now the comments are lying to you.
// assign 5 to foo foo = 4;
You should have written a code commenter that reads uncommented source code and comments it for you!
-
@ben_lubar said in For posterity.:
@anotherusername said in For posterity.:
(.?\d){14}
While this does match 14 digits after a decimal point, it also matches like a trillion other things.
It matches any 0 or 1 characters followed by a digit, 14 times. This effectively skips over everything that isn't a digit (so long as it's a lone non-digit, not 2 in a row) and returns true if it finds 14 digits. The first half of the regex already verified that the non-digits were present and in the correct locations. And since the beginning and end of the regex are pinned to the beginning and end of the string, it can only match strings that are 19 characters.
It matches the datetime format as well as
/^\d\d\d\d-\d\d-\d\dT\d\d:\d\d:\d\d$/
does, while being a hell of a lot more readable. And it matches it better than checking the length and 2 select characters does.While, sure, something that looks exactly like a datetime but encodes an invalid date will break, it would've broken in the original code. The only way around that would be to go with something like what @darkmatter suggested, which would probably be the best way to solve it in certain cases (or even in the general case), but the regex could also be perfectly adequate in some situations.
-
@anotherusername said in For posterity.:
I kinda like:
/^(?=....-..-..T..:..:..)(.?\d){14}$/
Why not just pitch it into an actual datetime parser?
-
@dkf you mean actual datetime parsers aren't made from regex soup?
-
@Jaloopa No, but they're canned regex soup. That makes all the difference. :)
-
-
@accalia
Seems like your noodle is jammed
-
@Luhmann said in For posterity.:
@accalia
Seems like your noodle is jammedindeed. it's stuck in :maximum: :mode:
-
@eskel Maybe both lines show as changed because the line ending (CR vs. CRLF) of the first line changed?
-
@bbolli Nah, we think this is a quirk of p4diff. Every reported change always includes one additional line at the top. Weird but otherwise harmless.
-
@dkf said in For posterity.:
@anotherusername said in For posterity.:
I kinda like:
/^(?=....-..-..T..:..:..)(.?\d){14}$/
Why not just pitch it into an actual datetime parser?
@Jaloopa said in For posterity.:
@dkf you mean actual datetime parsers aren't made from regex soup?
You mean like
/^(((?!00)([02468][048]|[13579][26])00|\d\d(?!00)([02468][048]|[13579][26]))-02-29|\d{4}-(02-(0[1-9]|1\d|2[0-8])|(0[469]|11)-(0[1-9]|[12]\d|30)|(0[13578]|10|12)-(0[1-9]|[12]\d|30|31)))T(24:00:00|([01]\d|2[0-3]):[0-5]\d:[0-5])\d$/
-
@error said in For posterity.:
every statement had to be commented no matter how mundane.
You should've put irrelevant stuff in.
//go mavs!
or whatever.
-
@loose said in For posterity.:
In my dim and distant past I recall being told that under certain circumstances such declarations needed to be declared twice.
The only possible reason I can see for that wouldn't involve scalars at all, but otherwise-circular struct definitions:
struct a { struct b *bargle; }; struct b { struct a *argle; };
is no good, replace with:
struct b; struct a { struct b *bargle; }; struct b { struct a *argle; };
ETA: Fixed a mistake.
-
@FrostCat Doesn't work like that, even in the second version
struct a
is not completely defined because it's size is not known. It does work as you describe if the members are pointers.
-
@PleegWat Guh. Yeah, I meant those to be pointers.
-
@FrostCat I use that in a bunch of places with tree-like data structures, when the child item has a different type from the parent.
struct child; struct parent { struct child * children; /*...*/ }; struct child { struct child * next; struct parent * parent; /*...*/ };
Note you don't need it if the parent and child have the same type; the following will just work on its own:
struct node { struct node * parent; struct node * next; struct node * children; /*...*/ };