• Wierdo (unregistered)

    Frist to care

  • P (unregistered)

    TRWTF is someone apparently care enough about this to submit it to TDWTF

  • (nodebb)

    This is probably fine, the StopCondition(int count) overload, as opposed to StopCondition(int ID), is called, which stops N conditions in one call.

  • Kanitatlan (unregistered)

    On the other hand, this is an almost perfect story to illustrate the fact that correct software only occurs because someone cares and never just because of process. If process gives everyone the excuse to not care you get crap software.

  • Foo AKA Fooo (unregistered) in reply to Wierdo

    First good "first" comment this year.

  • Brian (unregistered)

    Looks like it would work just fine, if the intent of the code is to remove the last condition in a list. Given that, in this snippet, it's apparently possible for a client to have more than one condition, there's no selector other than client ID to get the condition ID, and even though the function is named "count" the value it returns is treated as an ID, it would seem reasonable (not that I'm actually agreeing with this sort of design, just saying it's possible) to assume that the function returns some kind of encoded value which is then decoded in StopCondition.

    Or maybe I'm just overthinking this and the original coder is a moron.

  • RLB (unregistered) in reply to Brian

    Looks like it would work just fine, if the intent of the code is to remove the last condition in a list.

    IFF the ID of the condition is its position in the list, and there is only ever one client. Neither of those seems particularly likely to me.

  • Richa Bhimani (unregistered)

    I am glad that I found such an amazing article and it is really informative so thanks a lot for sharing it with us.

  • (nodebb)

    Honey Badger don't care either.

  • (nodebb)

    Just bad knowledge of English. Can see such things at some places in our code base, where "count" should actually be "number" or "id". Writing readable code? For whom? Don't care.

  • (nodebb)

    Actually this is a good reminder that type safety can be used to avoid these problems. If you need to refer to an entity, use the entity class, not an integer. For URLs, use System.Uri. For file paths, FileInfo and DirectoryInfo. The rule is, if you can conceivably use a type which isn't a primitive, use it; integers and strings should be reserved only for when things like counts/indexes, and names/descriptions, especially.

    Addendum 2019-09-18 11:30: Not especially - respectively.

  • David Mårtensson (unregistered)

    I have seen a similar error in an old internal admin gui.

    Fortunately is was not related to customer data.

    It was a list of items with a delete button for each item.

    When you clicked the button, the item was removed. BUT if you reloaded the page afterwards (this was early webforms) it suddenly deleted the next in the list, and the next, until all items after the original was deleted and then it crached.

    Turns out it did not post back the id of the item to remove, it posted back the position in the list and the items was stored in the viewstate that was encrypted in a hidden field.

    And the reload posted the same index, and as long as viewstate still containd enough items, it just picked the current one in the list, deleted it, fetched the list again from the database and updated viewstate.

    Once there was less items in the list, an out of bounds exception triggered instead.

    Next version of viewstate started using unique identifiers instead of index for looking up auxiliary data ;)

  • (nodebb)

    The ops team monitors the logs? Great idea, but what kind of org would have that luxury?

  • (nodebb) in reply to bradwood

    My Big Important Work Project sends me an e-mail whenever there's an actual uncaught error (mostly "invalid viewstate", presumably because their connection is slow/flaky), or the metric data shows an incomplete report (mostly slow/flaky or "we open a new tab after server-side validation and their pop-up blocker freaks").

  • (nodebb)

    Which caused a different problem when someone fired up a security suite and suddenly I got ten thousand e-mails in an hour, bogging the e-mail system down like whoa. Had to schedule it for a specific time period and throttle the e-mails down from "every 5 minutes" to "every hour" during that window.

  • Developer Dude (unregistered) in reply to Mr. TA

    Yes, this is one reason why I prefer strongly typed languages; if properly written, each method would either return or take a typed return or parameter value. Then the compiler/interpreter/et. al. would catch the error before the code would go into a repo. I.E., compiler time errors are preferable to runtime errors.

  • sizer99 (google)

    This is the real answer to most questions about WTFs in the computer world. 'They just didn't care.'

    Why don't Snapchat's messages really disappear? They just didn't care.

    Why are critical medical devices unsecured, to the point where you can kill people? They just don't care.

    Why is every IoST device completely insecure? They just don't care.

    Why do all these giant companies that hold all your personal data inevitably leak all the data? They just don't care.

    And the payouts / fines are just slaps on wrist, so they still don't care.

  • Que? (unregistered)

    Great example where typed languages (as long as you make the effort to create them...) are less error prone. ie:

    int conditionId =  _monitorConditionManagement.GetActiveConditionCountByClient(clientIdentityNumber);
    _monitorConditionManagement.StopCondition(conditionId) // error ... expects ConditionId type
    
  • D. OH (unregistered)

    Maybe the conditionId is stored as a running number and by doing _monitorConditionManagement.GetActiveConditionCountByClient(clientIdentityNumber); it would the last condition id?

  • Alexander Malfait (unregistered) in reply to Mr. TA

    Indeed, and even when you can't use the actual entity, you can still pass an ID in a typesafe way.

    We have a class Ref<T>(Class<T> entityClass, long entityId) for this purpose.

  • Little Bobby Tables (unregistered) in reply to emurphy

    I've long been a fan of the email technique. The main problem can be debunked:

    "I'm getting too many emails and it's clogging my system." Then the company should ensure that the developers fix the software so as to do something sensible with those emails. If they are genuinely reporting a problem to be attended to, then that means there is a problem to be attended to, so go and attend to it. If they are not reporting something which is actually a "problem" as such, then handle the error programmatically.

    Errors which are the result of a programming mistake or carelessness or oversight or short-cut should be addressed. That's why they are logged. If an "error" is genuinely not a problem then swallow it.

    A case in point is a log file that gets filled with stackdump in an application I regularly use. It's returning a class missing error. "Oh, not to worry about that," says the developer, "that class is optional. You don't need it and we don't package it." So stop logging the stackdump and change its level to informational, doofus!

  • Code Refactorer (unregistered)

    Dear Remy,

    the post from Richa Bhimani is a covered advertising. Can you please delete it? The advertising is the link under " thanks a lot". And second, it says nothing...

  • Ruther Rendommeleigh (unregistered)

    Am I the only one who thinks that with a more concise naming scheme, and possibly a better separation of concerns, the mistake might not have happened in the first place? It's easy to miss one word in a 100-character method call. Something like, say, GetClient(clientID).GetActiveConditions().count seems much more obvious.

  • (nodebb) in reply to Little Bobby Tables

    That might depends on how easy it is to identify which missing class is causing the error. Parsing the error message to determine that isn't ideal.

  • MiserableOldGit (unregistered)

    I must say, normally when I see that sort of thing (and I see it a lot) it's just a badly named function.

  • (nodebb)

    Make your IDs non-numeric. It exactly guards against all this sort of idiocy. (The way you do it varies from language to language, but the important bit is that you don't want to multiply an ID by anything, or compute an ID by dividing a user-provided value by 17…)

  • Git Commit Police (unregistered)

    “You really shouldn’t fix two unrelated bugs in the same commit”

    This is good advice. Make two commits and have them reviewed together.

Leave a comment on “You Can Take Care”

Log In or post as a guest

Replying to comment #508350:

« Return to Article