• (cs)

    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.

  • Mrs. Matt Westwood (unregistered) in reply to Matt Westwood
    Matt Westwood:
    Whee! I've got a date!
    And a divorce.
  • Anon (unregistered) in reply to Anonymous Paranoiac
    Anonymous Paranoiac:
    Fredegar:
    In C# is it considered good practice to set strings to NULL instead of empty?

    TRWTF is the management policy of rating programmers by Lines Of Code produced. This is why they have an entire contingent of redundant scrubbers, instead of a one line function for all strings.

    Alternately,

    Management: "What do you mean you're scrubbing all data the same way?! Everybody knows that you can't scrub a customer name the same way you scrub a phone number - they're completely different! Now stop being lazy and go back and do it right this time!"

    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.

  • The Fury (unregistered) in reply to Askimet
    Askimet:
    Without knowing C#, would the following work?
    public static string ScrubString(string stringToScrub) {
      return (stringToScrub == string.Empty ? null : stringToScrub);
    }
    

    No that wouldn't work, try this:

    public static string ScrubString(string stringToScrub) {
      return string.Empty ? null : stringToScrub;
    }
    
  • The Fury (unregistered) in reply to The Fury
    The Fury:
    Askimet:
    Without knowing C#, would the following work?
    public static string ScrubString(string stringToScrub) {
      return (stringToScrub == string.Empty ? null : stringToScrub);
    }
    

    No that wouldn't work, try this:

    public static string ScrubString(string stringToScrub) {
      return string.Empty ? null : stringToScrub;
    }
    

    Ignore me.

  • (cs) in reply to Matt Westwood
    Matt Westwood:
    There is a certain therapeutic satisfaction in having a job that consists of cleaning up shit code. It's like an academic exercise: [snip] b) indicate particular points of interaction at which customer satisfaction will be improved, and estimate the probability that, as a result of your having considerably improved her daily workload, you'll get to boff that tasty little brunette in Customer Services."

    Hey, I like blondes, you insensitive clod!

  • (cs) in reply to Mrs. Matt Westwood
    Mrs. Matt Westwood:
    Matt Westwood:
    Whee! I've got a date!
    And a divorce.
    Finally she gets the fucking message.
  • (cs) in reply to Oslo
    Oslo:
    ObiWayneKenobi:
    they just wrote the first thing they could think of to finish the task and then move on to the next one.

    Isn't that just how TDD ends up working?

    No, the flow of TDD is "Red, Green, Refactor"

    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.

  • TheOldNewFuManChu (unregistered) in reply to noone

    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.

  • (cs) in reply to RFoxmich
    RFoxmich:
    TRWTF is a full rewrite rather than fixing

    http://www.joelonsoftware.com/articles/fog0000000069.html

    rewriting a service != full rewrite

  • (cs) in reply to faoileag
    faoileag:
    QJo:
    I can't even comprehend what was the motivation behind this code's existence.
    A bad attempt at memory consumption optimization? Strings in C# are reference types.
    string a = "";
    string b = a;
    
    String b now has a reference to string a; string a can't be garbage collected.
    string a = "";
    string b = ScrubbingBubble.ScrubCustomerName(a);
    

    String b now is null, the connection with string a has been severed. String a can now be garbage collected.

    Still a rather costly way to do this;

    string b = string.IsNullOrEmpty(a) ? null : a;
    

    would be more concise and would not necessitate the creation of a temporary object.

    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.

  • (cs) in reply to Askimet
    Askimet:
    Without knowing C#, would the following work?
    public static string ScrubString(string stringToScrub) {
      return (stringToScrub == string.Empty ? null : stringToScrub);
    }
    

    It's not how it was written the was the problem, it's the purpose of the function itself that is the WTF.

  • (cs) in reply to Fredegar
    Fredegar:
    In C# is it considered good practice to set strings to NULL instead of empty?

    TRWTF is the management policy of rating programmers by Lines Of Code produced. This is why they have an entire contingent of redundant scrubbers, instead of a one line function for all strings.

    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.

  • (cs) in reply to Matt Westwood
    Matt Westwood:
    Whee! I've got a date!

    I wish you and the future Mrs. Westwood well.

  • (cs) in reply to chubertdev
    chubertdev:
    Matt Westwood:
    Whee! I've got a date!

    I wish you and the future Mrs. Westwood well.

    Thx bro, I'll save you some wedding cake.

  • J (unregistered) in reply to Askimet
    Askimet:
    Without knowing C#, would the following work?
    public static string ScrubString(string stringToScrub) {
      return (stringToScrub == string.Empty ? null : stringToScrub);
    }
    

    Way to miss TRWTF.

  • (cs) in reply to DrPepper
    DrPepper:
    No, the flow of TDD is "Red, Green, Refactor"

    We now come to the part of the show called "If it ain't broke, you're not trying!"
  • (cs)

    If this is the most egregious excerpt you could fine, then you have a very cushy job. Stop complaining!

  • (cs) in reply to JimM
    JimM:
    Fredegar:
    In C# is it considered good practice to set strings to NULL instead of empty?

    Depends what you're going to do with the string :p

    No, seriously. It's entirely contextual. AFAIK there's no real advantage one way or the other (except perhaps for consuming code, which would know whether it needed to check for null, Empty, or both. Then again, that would rely on the code documenting whether it returned null, or Empty, or either (arbitrarily), which given the quality of this code, seems unlikely).

    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.

  • anon (unregistered) in reply to Steve The Cynic
    Steve The Cynic:
    zbxvc:
    Steve The Cynic:

    The new codebase was largely platform independent, aside from a portability layer, and single threaded. I worked on the architecture of it with a colleague who had previously worked on routers at DEC, and as a result of his experience, we had a model that never completed anything in-line, but posted delayed callbacks (overlapped I/O style) to all requests. No code was allowed to block for any reason. The new code base came up and working in about half the time the previous one took, and just worked. It never deadlocked, nor did we ever have problems with long-term leaks, nor objects dying before their time.

    Could you share some more details about the model? I'm interested mostly because:

    • in my experience (probably different domain, or just wrong kind of experience ;) ) most multithreading issues turned out to be caused by not doing as much as possible "inline" (and using excessive number of threads, but this aspect seems common to both cases, yours and mine),

    • I don't quite understand (and would like to) how it was single threaded and couldn't block: the main() function would need to use 100% CPU or return immediately when there is nothing to do, wouldn't it? (Or is it how it was supposed to work, i.e. do what is to be done and exit? Perhaps I wrongly assumed that it was a kind of a server app that was supposed to react to external requests...)

    In essence, main() did some application-specific setup including opening listening sockets(*) or outbound client->server connections, then entered a big loop that blocked waiting for network traffic to arrive. main(), or rather the callback scheduler, was, of course, allowed to block, but callbacks were not. If your callback needed to send a message and wait for a response, or connect a TCP connection, or whatever, it initiated the action, then returned back to the scheduler. When the receive completed, or the TCP connection completed/failed, the network layer(*) sent back a notification in the form of a callback. The scheduler had timers that you could get notifications from, using, as you might guess, callbacks.

    There was a callback proxy-stub generator that read descriptions of interfaces so that you didn't have to write the boring boilerplate code to post result notifications back to calling objects.

    There are four main sources of threading-specific problems that I've seen in threaded code:

    • Deadlocks of all types, including the notorious self-deadlock, where the thread that must process the request you've posted is the one you're in, but you've blocked waiting for it to be processed.
    • Not enough locking causing all manner of nasty race-condition bugs, and often leading to the third problem:
    • Using objects that have been deleted by other threads. This is more of a problem with C and C++ than with, say, Java or C#, but it's a problem nonetheless.
    • In attempting to prevent the third category of problems, you overshoot and fail to delete the objects when you should.

    It's possible, as we did in the old system, to have all four categories of bug, even in the same pair of objects.

    (*) The network layer was designed to be saner to program than the normal sockets API, with different objects for TCP connections, TCP listeners, and UDP listener-senders. Inside, it used a platform-specific layer to adapt to the specifics of the sockets API on the different platforms (or, even, the fact that a particular platform had a different API). There were subtle differences between the BSDs and Linux because of silly things like FreeBSD's unique (among the platforms we tried) behaviour of completing to-localhost connect() calls for non-blocking TCP sockets in-line rather than failing with EWOULDBLOCK.

    You just described node.js.

    Captcha: aptent, an apt tent.

  • zbxvc (unregistered) in reply to Steve The Cynic
    Steve The Cynic:
    In essence, main() did some application-specific setup including opening listening sockets(*) or outbound client->server connections, then entered a big loop that blocked waiting for network traffic to arrive. main(), or rather the callback scheduler, was, of course, allowed to block, but callbacks were not. If your callback needed to send a message and wait for a response, or connect a TCP connection, or whatever, it initiated the action, then returned back to the scheduler. When the receive completed, or the TCP connection completed/failed, the network layer(*) sent back a notification in the form of a callback. The scheduler had timers that you could get notifications from, using, as you might guess, callbacks.
    Thanks!

    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...

    Steve The Cynic:
    There are four main sources of threading-specific problems that I've seen in threaded code: * Deadlocks of all types, including the notorious self-deadlock, where the thread that must process the request you've posted is the one you're in, but you've blocked waiting for it to be processed.
    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).
    Steve The Cynic:
    * Not enough locking causing all manner of nasty race-condition bugs, and often leading to the third problem: * Using objects that have been deleted by other threads. This is more of a problem with C and C++ than with, say, Java or C#, but it's a problem nonetheless. * In attempting to prevent the third category of problems, you overshoot and fail to delete the objects when you should.
    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?

  • (cs) in reply to savar
    savar:
    If this is the most egregious excerpt you could fine, then you have a very cushy job. Stop complaining!
    Some of us just wish that what we had to put up with was so nice. Mere waste? Luxury!

    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).

  • (cs) in reply to zbxvc
    zbxvc:
    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?
    I'd guess a chunk of both. Too many programmers over-use threading, and it's horribly easy to get massively wrong. It's much easier to get linear code right. Fully parallel code (i.e., with preemptive multitasking, such as you get with multiple CPUs) is really tricky unless you take special steps to tame it, e.g., moving to a system based on strong isolation and message queues; it can still have deadlock problems, but it's a heck of a lot easier to analyse. Such apps tend to end up looking like a collection of services that all communicate only relatively rarely, but they do scale out fairly well (routing messages to other processes, even remote ones, isn't too hard). The fully-shared model used by C and C++ and Java and C# and Python and… That doesn't scale nearly so well; it's debugging all the weird interactions that gets you.

    There's really no substitute for actually thinking about what you're doing. Experience tells me that most folks don't really believe that.

  • (cs) in reply to savar
    savar:
    JimM:
    Fredegar:
    In C# is it considered good practice to set strings to NULL instead of empty?

    Depends what you're going to do with the string :p

    No, seriously. It's entirely contextual. AFAIK there's no real advantage one way or the other (except perhaps for consuming code, which would know whether it needed to check for null, Empty, or both. Then again, that would rely on the code documenting whether it returned null, or Empty, or either (arbitrarily), which given the quality of this code, seems unlikely).

    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.

    "Is it understandable for less-than-great and/or time-pressured programmers? Yes."

    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.

  • Paul Neumann (unregistered) in reply to Matt Westwood
    Matt Westwood:
    On the other hand, if she is open to listening to reason, and would prefer a straight-up guy who actually knows how to fix things, not just fuck them, then hey, how about it, babe?
    So, she needs fixing?
  • (cs)

    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.

  • Johnny (unregistered) in reply to EvilSnack
    EvilSnack:
    RFoxmich:
    TRWTF is a full rewrite rather than fixing

    http://www.joelonsoftware.com/articles/fog0000000069.html

    A full rewrite is not always evil.

    Sometimes the initial assumptions made during the design phase are utterly wrong and have led development down a path where even minor changes cause problems.

    Sometimes a regime of short-sightedness has led to an application being patched and re-patched, without regard for the overall design, to the point that the overall design is hardly discernible.

    Indeed. TRWTF is believing that there can ever be a rule that something is always right or something is always wrong.
  • Jim (unregistered) in reply to Pock Suppet
    Pock Suppet:
    EvilSnack:
    RFoxmich:
    TRWTF is a full rewrite rather than fixing

    http://www.joelonsoftware.com/articles/fog0000000069.html

    A full rewrite is not always evil.

    Sometimes the initial assumptions made during the design phase are utterly wrong and have led development down a path where even minor changes cause problems.

    Sometimes a regime of short-sightedness has led to an application being patched and re-patched, without regard for the overall design, to the point that the overall design is hardly discernible.

    This. Sometimes the original code is, in fact, a moldering pile of lice-infested crap with a side of open pustules, and years of feature requests have merely stapled new appendages to the existing rotting corpse. Sometimes the person who wrote the original code had never heard of best practices or even good practices, thought that "code reuse" meant "copy-paste", delighted in over-complicated systems with dozens of tightly-coupled interlinked moving parts, enjoyed storing variables in global space in one file and using their contents in another, and never met a Rube Goldberg machine they couldn't top with any random 15-line snippet from their current project. Some programmers -suck-, and the rest of us have to deal with their code base.

    Also, programming environments become...outdated. The first programming job I had was maintaining code written in and around a (multi-valued, not relational) database system that was designed in 1968. It uses upper ASCII characters for field delimiters, and that caused great Fun when dealing with non-American customers. It's limited to ASCII characters, so no Unicode - in the event that Unicode support becomes necessary (which is pretty likely, given their customer base), the entire code base will require a rewrite on some other platform.

    MOST programmers suck (yes even a lot of you (us) pointing it out in these comments). Humans naturally assume that they are above average - and can rarely see their own failings. I would be surprised if there is a single person here who hasn't (and doesn't continue to) find inefficient, misguided or downright daft pieces of code they've written in the past (and as we grow more experienced we continue to find it - because we start to understand better what we're actually doing, so we can recognise what we've done wrong in the past - even the recent past). Often the issue is hacking functionality because you're not aware that something already exists that does roughyl what you want to do (probably way more efficiently) etc, etc. With programming (as with all things) most people who claim to be awesome at it are simply oblivious to gaps in their knowledge - most people who genuinely are awesome at it recognise their limitations, and although they might consider themselves reasonable, would likely focus on their defficiencies...

    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

  • Oxy (unregistered) in reply to Steve The Cynic
    Steve The Cynic:
    We also had one on VxWorks, which was stuck on an antique version of gcc (2.95 ffs!) that didn't handle anonymous namespaces correctly.
    Anonymous namespaces? What do you call those?

    Yours filenotfoundedly, O. Moron

  • Meep (unregistered) in reply to QJo
    QJo:
    I can't even comprehend what was the motivation behind this code's existence.

    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.

    Shall we just make sure that when anyone enters nothing into a mandatory field we invoke a null pointer exception?

    TRWTF is that your strongly-typed language allows any reference type to contain a value that might cause your program to fail catastrophically.

  • Cheong (unregistered)

    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.

  • (cs) in reply to Paul Neumann
    Paul Neumann:
    Matt Westwood:
    On the other hand, if she is open to listening to reason, and would prefer a straight-up guy who actually knows how to fix things, not just fuck them, then hey, how about it, babe?
    So, she needs fixing?
    Oh come on. *All* women need *something* fixing, either if it's her bathroom cabinet onto the wall or her as-yet unsatisfied sexual frustration, there's always *something* you can do to improve her life.

    Whether it goes the other way is debatable.

  • (cs) in reply to bubble
    bubble:
    public static string ScrubFrist(string comment) { string newComment = string.Empty; if (string.IsNullOrEmptyOrFrist(comment)) newComment = null; else newComment = comment; return newComment; }

    FTFY

  • (cs) in reply to Oxy
    Oxy:
    Steve The Cynic:
    We also had one on VxWorks, which was stuck on an antique version of gcc (2.95 ffs!) that didn't handle anonymous namespaces correctly.
    Anonymous namespaces? What do you call those?

    Yours filenotfoundedly, O. Moron

    OK, I'll bite. Or feed the troll, or whatever.

    A C++ anonymous namespace looks like this:

    sample.cpp:
    namespace
    {
      int theAnswer = 42;
      SomeClass someThing( 3, 4, 5 );
    }
    For those in the audience who aren't familiar with this aspect of C++, the code above is the equivalent of this:
    sample2.c:
    static int theAnswer = 42;
    static SomeClass someThing( 3, 4, 5 );
    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:

    collect2_output.c:
    void callConstructors()
    {
      extern SomeClass magicIdentifierForsomeThing;
    

    constructor_for_SomeClass( magicIdentifierForsomeThing, 3, 4, 5 ); }

    Unfortunately, the magic identifier included the base filename (sample.c) without modification, so the result would be:

    collect2_output.c:
    void callConstructors()
    {
      extern SomeClass sample.c__someThing;
    

    constructor_for_SomeClass( sample.c__someThing, 3, 4, 5 ); }

    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.

  • (cs) in reply to zbxvc
    zbxvc:
    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?
    As dkf said, a bit of both. It was harder to write the sort of bad code that we had had in the COM/DCOM version of the code, because we didn't have threads and we did have the object lifetime management system, and certain other things to reduce simplestupid errors. The slight unfamiliarity of the event-driven model forced people to think about what they were doing, which always helps produce better code. There were also debugging tools that helped resolve certain kinds of leaky bugs - they collectively fell down and worshipped us when we (the platform layer team) added a feature where you could poke a running program and get a dump of its refcounted objects. (Like the one that was supposed to be empty at the end of main()).

    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.

  • El_Slapper (unregistered)

    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.

  • QJo (unregistered) in reply to El_Slapper
    El_Slapper:
    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.

    This.

    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.

  • xTr1m (unregistered) in reply to Askimet

    Even easier:

    public static string ScrubString(string stringToScrub) {

    return stringToScrub ?? string.Empty; }

  • (cs) in reply to xTr1m
    xTr1m:
    Even easier:

    public static string ScrubString(string stringToScrub) {

    return stringToScrub ?? string.Empty; }

    No, you've changed the behaviour, because the null-or-empty return value should be null, not the empty string.

  • (cs) in reply to zbxvc
    zbxvc:
    Thanks!

    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...

    Sorry, meant to answer this earlier. The application was an H.323 proxy with a slightly hairy (and not at all relevant) variation. So it would listen for connections, and each time one arrived it would spawn a connected TCP socket for the new connection, and handle incoming data on it. Each time a complete message arrived (no matter how many packets it took, the platform layer delivered the result to the application, which would crank the handle on a state machine to handle the specific message, send out any outbound messages, then return. As the sends completed, they would notify completion as well. So the increment of work wasn't a request in the web-server sense, but the general principle applied.

    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.

  • Paul Neumann (unregistered) in reply to Matt Westwood
    Matt Westwood:
    Oh come on. *All* women need *something* fixing, either if it's her bathroom cabinet onto the wall or her as-yet unsatisfied sexual frustration, there's always *something* you can do to improve her life.

    Whether it goes the other way is debatable.

    So, if it does go the other way for a majority of men, would that make it mass-debatable?

  • anonymous (unregistered) in reply to Matt Westwood
    Matt Westwood:
    Paul Neumann:
    Matt Westwood:
    On the other hand, if she is open to listening to reason, and would prefer a straight-up guy who actually knows how to fix things, not just fuck them, then hey, how about it, babe?
    So, she needs fixing?
    Oh come on. *All* women need *something* fixing, either if it's her bathroom cabinet onto the wall or her as-yet unsatisfied sexual frustration, there's always *something* you can do to improve her life.

    Whether it goes the other way is debatable.

    Not to mention they're also born with these things called "tubes"...

  • Hannes (unregistered) in reply to RFoxmich
    RFoxmich:
    TRWTF is a full rewrite rather than fixing

    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.

  • C (unregistered) in reply to RFoxmich

    I don't know. Joel makes a lot of dodgy assumptions there. This one's my favorite:

    [Old Code] has been tested. Lots of bugs have been found, and they've been fixed.

    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."

  • (cs) in reply to pay me to comment
    pay me to comment:
    ObiWayneKenobi:
    Ignorance? Arrogance?
    Money.

    yes. nobody wants to spend on thing that is doing job 95% of time.

  • Anone (unregistered) in reply to Meep
    Meep:
    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.)

    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).

  • sivilay (unregistered) in reply to MP79

    Moreinformation please contact me:

  • sivilay (unregistered)

    More information please contact me:

  • Morecambe (unregistered) in reply to Wrexham
    Wrexham:
    Anonymous Paranoiac:
    If you can't spot WTFery (or, at minimum, things that could have been done better) in your own well-aged code, then you seriously need to work harder on improving your skills.
    Nonsense, every line of code I've ever written was perfect first time.
    Just not necessarily in the right order.
  • Azarien (unregistered)

    The real WTF is that in C# the empty string is not equal to null. It could have.

Leave a comment on “A Scrubbing Bubble”

Log In or post as a guest

Replying to comment #:

« Return to Article