- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
This might not be that big of a WTF depending on how it was used. What if the purpose was to sanitize the data for database insertion? If the field is a nullable varchar, this method helps to eliminate empty strings. Doing it this way, the data could be compared against NULL instead of performing a length operation on the data which would be incredibly slower.
Sure the method could have been written a bit better, but I don't see any obvious WTFs given the back story.
Admin
Admin
Today's Dilbert seems apt:
http://www.dilbert.com/fast/2013-10-17/
Seriously, "Akismet says your comment was spam". Fuck you Akismet. TRWTF is that this STILL hasn't been fixed.
Admin
No that wouldn't work, try this:
Admin
Ignore me.
Admin
Hey, I like blondes, you insensitive clod!
Admin
Admin
Because the refactor part comes after you have a test harness in place, you should (must) investigate the code you wrote and refactor it to improve it; the tests are there to ensure you didn't break anything.
Someone didn't do their refactoring right.
Admin
Except the issue here is that you're introducing extra complexity into the problem, so it should stay as the test against string.Empty.
The choices are string.Empty, null, string. By returning null or [null|string] you're only returning two things potentially.
Therefore, the simplest test, and the one that most directly indicates your assertion (that "" should be translated to null) is more clear than adding in the additional test, even if it's more enterprisey. By returning null, you've already indicated that null is an acceptable return from the function, so a null input is acceptable as a passthrough value.
Don't complicate shit by using functions when a simple comparer works fine.
Captcha: distineo. It's my distineo to correct things which don't really require correction. I'm a maintenance dev tho, so ... ymmv.
Admin
rewriting a service != full rewrite
Admin
holy premature optimization, Batman. you're changing how a function works to try and optimize probably the most lightweight part of the service. that's needlessly opening the door for bugs.
Admin
It's not how it was written the was the problem, it's the purpose of the function itself that is the WTF.
Admin
As other people stated, it's definitely based on context. I usually use the same context, though: null means that there is no value, that the variable is undefined. An empty string means that it does have a value, the value is empty.
Admin
I wish you and the future Mrs. Westwood well.
Admin
Admin
Way to miss TRWTF.
Admin
Admin
If this is the most egregious excerpt you could fine, then you have a very cushy job. Stop complaining!
Admin
Presumably, these objects are being mapped into a relational database, where the difference between an empty string and null is very critical and will break some queries. They probably expected (or actually had) data that would never put an empty string for the customer name field, and that worked fine until one day it suddnely didn't work.
So they got really paranoid and wrote a bunch of scrubbing functions to fix it, but since they didn't know if they would discover other hidden requirements for other fields in the future, they gave each field it's own separate scrubber function so each one could be modified separately.
Is it a stupid pattern? Yes.
Is it written poorly and a waste of time? Yes.
Is it understandable for less-than-great and/or time-pressured programmers? Yes.
Admin
You just described node.js.
Captcha: aptent, an apt tent.
Admin
So in essence the difference seems to be that you did not actually need concurrency, i.e. processing a request was never taking enough CPU time to make other requests wait unacceptably long; is that right? Otherwise, I imagine, you would need to manually divide the long-lasting processes into chunks, creating a kind of cooperative multitasking at the application level...
Yes! Seen too many times. But what usually helped was to replace "post and wait" code into "just do it right here" code (and to make sure the locking is right). The tricky part about locks, I think (apart from having an understanding of how to use them) is finding the balance: use few wide-area locks and you may kill performance by making otherwise unrelated tasks wait for a common lock (although using read/write lock instead of plain mutex/semaphore can be of some help here), use many small-area locks and you increase the probability of a) not covering some area, b) typical deadlock when you end up acquiring a set of locks in two threads in different order.However, I found it a bit surprising when I once realized that - in the code I had been working with - the object deletion issue was not really a common problem.
But back to your story: I'm wondering if the new system worked much better than the old one mostly because of the architecture change, or maybe mostly because the people designing and implementing it were simply better developers? I.e. is there any aspect that make the "threading" approach inherently unsuitable (or significantly harder to do properly) for that particular task, or maybe it could work as well as your new system if it was coded by people who knew what they were doing?
Admin
The current bane of my life is a build system that can be best described as “Maven and OSGi decided to get together and have a love child, but that child was both retarded and severely autistic.” It's got a lovely mix of automated steps (that require a strangely-configured Jenkins CI service to get right) and manual steps (that require someone to follow a script and manually edit a file to splice in the results) that manages to resist being reduced. It also binds absolutely everything to exact version numbers; update a core module? Everything else has to be edited to include new version numbers and rebuilt from scratch. Ugh!
We know it's bad. We've got a project to replace all that stuff (though not a total from-scratch rewrite).
Admin
There's really no substitute for actually thinking about what you're doing. Experience tells me that most folks don't really believe that.
Admin
No it bloody is not. Five bloody lines of cruft which you have to look at carefully before you say, jesus! All it does is turn an empty string into a null! Is that it? and mentally (and physically if you have any cojones at all) replace it with a ternary if.
And then couple that with the fact that you have to wade through the same shit for half a dozen other fields, all of which implement exactly the same thing, and that's plenty of wasted brain-churn for this time-pressured programmer.
Admin
Admin
Meh, I'm not that offended by this. There's probably other code that expects a "null" when the thing isn't set, and doesn't support empty strings.
Sure, those points should call string.IsNullOrEmpty to be safe, but this might have been a case of somebody adding it in one place instead of a million.
Admin
Admin
As for outdated environmnets/languages/paradigms/whatever - this is one of the biggest difficulties. People who were great using cobol on the mainframe, or who grew up writing shit in assembler that we can barely do with the latest and greatest tools at our disposal often resist (not even consciously, I suispect sometimes) new technologies - or use them badly because to their way of thinking (especially with the assembler crowd) they understand much better than the language designers what's going on, so they should be able to hack away a better solution....
Outdated
Admin
Yours filenotfoundedly, O. Moron
Admin
Yeah, I usually do the opposite in Java of replacing any null references to String with an empty string.
It might be that they're using Oracle, which has the uncommon semantics that a zero-length varchar is NULL. (It's actually easier to work with and more consistent, in practice. It's regrettable that SQL went the other way.)
If you take the (correct) view that the DBMS represents the canonical version of your business logic, it makes sense to make your application conform to the way the DBMS works rather than the other way around.
TRWTF is that your strongly-typed language allows any reference type to contain a value that might cause your program to fail catastrophically.
Admin
Once upon a time we changed the grid we used and found it incorrectly set string fields to empty instead of null, which our store procs expects for null data.
In that case it make sense to beat "known source of error" to behave instead of diving in see of unknown code to change the value one by one. And I think it's high probability to experience this more than once for an old codebase.
Other than the poor sense of naming, it doesn't seems to be too much a WTF for me.
Admin
Whether it goes the other way is debatable.
Admin
FTFY
Admin
A C++ anonymous namespace looks like this:
For those in the audience who aren't familiar with this aspect of C++, the code above is the equivalent of this: That it, variables with only file scope.In gcc versions later than 2.95 and/or not on VxWorks, this does some handwaving under the covers, and generates identifiers for these variables, and away you go. 2.95 on VxWorks requires that you process the .o files with a thing called collect2 to produce a C file that you compile and link with your code. This file contains a single large function that, for each object with a constructor, calls the constructor at startup, and another function that calls the destructor.
In essence, for the variables above, you'd get this result from collect2:
Unfortunately, the magic identifier included the base filename (sample.c) without modification, so the result would be: And that doesn't compile so well in C.Using the "static" version instead worked just fine. Go figure.
Addendum (2013-10-18 04:20): Oops: that should be "sample2.cpp" in the second code box, of course.
Admin
And of course, the problem domain was better understood so we didn't lose so much time on acquiring that understanding. Even so, with the change in the architecture came a far more rapid stability. (So the greater experience with the problem domain reduced problem-domain bugs like misinterpretation of standards, and the system design of the new environment helped eliminate solution-domain bugs like deadlocks, unlocked data corruption, and object lifetime mismanagement.)
On the other hand, even though they were smart guys, I was still able to boggle them occasionally. Stepping through code that Visual Studio's debugger displayed only in assembler didn't (and doesn't) faze me in the slightest, and watching me do it was usually enough to inspire jaw-on-floor amazement on their part.
Admin
had to fully rewrite applications in my career. twice.
after those experiences, My understanding of Joel's advice is "keep the functional knowledge".
It means you can rebuild from scratch, but you have to be sure you are 100% functionally compatible with the old spaghetti. In one case, I made a dedicated testing program comparing the (vastly different technically) output of both, & stopped only when the only remaining differences were bug corrections.
Other rewrite was a no-brainer : we had brand new specs, that seemed similar, but were in fact different enough to justify a canning of not-so-bad-but-functionally-outdated-code.
I can't see how you can applu Joel's advice on purely technical elements.
Admin
Joel's advice on the subject in question is clearly aimed at management / strategy decision makers. I will concede that there are situations where it does make sense to rewrite from scratch (e.g. when porting a FORTRAN codebase on a VAX to a Java app on a more modern platform).
But I contend that you still do it incrementally. You don't just "throw it away and start again from scratch". You take it module by module. And if your code is so badly written that you genuinely can't abstract that structure, then you either have serious problems or you're just not trying hard enough.
Admin
Even easier:
public static string ScrubString(string stringToScrub) {
return stringToScrub ?? string.Empty; }
Admin
Admin
Call processing, which can in theory be handled in an ongoing basis, was broken up into a state machine that launched various other actions so as to be able to process each message of a call sequence in the correct way. Network reads were always delivered by callbacks so the the caller could handle the messages in a 100% consistent way.
So yes, it was a cooperative multitasker, with a strong emphasis on get in, do something, get out, no waiting around while doing it. (There was no equivalent of Win16's Yield() function, for example.) If you had a long compute-bound activity, so be it, but if you had a long receive-compute-send-receiveresponse-compute-reply sequence, you broke it up into receive-compute-send-return followed by receiveresponse-compute-reply-return in a separate callback.
Admin
Admin
Admin
I have rewritten a project once, because it was "a complete mess".
And that it was indeed. 3000 lines of code in the main class alone (on my rewrite, I only needed half of that), overuse of if-else-cases when a simple "switch" would have done the same in less lines. 8 text boxes on the GUI (it was a C# project) to store various strings in it and to reuse it later. A hidden button on the GUI (hidden beneath a layer of those 8 text boxes, 6 data grid views and 4 report viewers) that was called in the code with a "PerformClick()" to hide various GUI elements. ALL of the GUI elements still had their default names (button1, button2, datagridview1... and so on).
A rewrite was much simpler in this case.
Admin
I don't know. Joel makes a lot of dodgy assumptions there. This one's my favorite:
Only, IME, if you're sufficiently generous with the word "fixed" to include "wrapped up in 15-case branches to cover all the half-cocked braindead add-ons."
Admin
yes. nobody wants to spend on thing that is doing job 95% of time.
Admin
Seriously?
Take a typical address, which includes an address line 2 which tends to not be necessary.
From a normalization-to-the-point-of-insanity perspective, address line 2 should be in a separate table, and addresses without them should just not have an entry in that table. In that sort of case, it doesn't matter that null and a blank string are equivalent, because you don't have any blank strings. But does anyone actually go to that extreme?
Especially given that most different parts of an address may be optional, depending on how wide of an area they need to cover, which means a separate table for each of them.
And when it comes down to actually using the address, it being split across different tables is going to make using it slightly more complicated for no apparent benefit.
You could leave all the pieces in a single table and just deal with having to leave the columns nullable, but that does actually lose you very minor tangible benefits (cannot detect any queries which are written to accidentally set null into those columns, such as a broken insert query which has forgotten one of them) and is semantically improper (given that null indicates that the value is unknown, not that it's blank).
Admin
Moreinformation please contact me:
Admin
More information please contact me:
Admin
Admin
The real WTF is that in C# the empty string is not equal to null. It could have.