20

Found this in a code review today.

Technically, I guess, that is one way to fix a divide by zero error.

Comments
  • 4
    And what was your comment?
  • 2
    Well.... We are waiting... :D
  • 10
    @asgs

    Just found out that the bit of code that can/has raised the exception is expected. Users didn't want 'bad' data to appear in the report. TL;DR, the code summarizes data and part of the division (ex sales / totalsales), if there are no sales, don't include the row.

    When I asked, "Wouldn't that be easier to filter out in the report?" he said

    "Yes, I asked the same thing, but the manager and two business analyst looked at me and said 'It would be easier for us if you fixed the code so bad data didn't show up.' Wasn't worth arguing and I decided not to die on that hill."

    Good enough for me.
  • 8
    @PaperTrail even better would be to validate the inputs before doing Arithmetic ops
  • 9
    @PaperTrail I still believe the proper way to avoid showing bad data is avoiding unnecessary/wrong operations by checking data before. Sweeping dust under the rug is never a solution.
  • 3
    @asgs agreed unless the arithmetic operation would return a valid state no matter the input (i.e. the distance formula).
  • 3
    @TestInProd423 That's kind of the situation. Personally, *I* would have written it differently, but the outcome would be the same, so its not worth the effort. Likely this code will go for years without being changed (last update to the project was 2019). Let the next Monday-morning-quarterback pound their chest in 5 years, which likely I'll still be here, and I'll say "put it back in your pants Ryan. Fix the code and move on. Nobody cares."
  • 8
    Maybe at least explain it's like that in the comment. Otherwise someone is going to be very confused in the future.
  • 1
    But why would someone catch an exception if it could be prevented?

    How come they're expecting an error if they could fucking prevent it anyway?

    What happened to zero value check????
    ---- if (denominator != 0) {
    // continue your code...
    }
  • 1
    It's like wrapping a whole program with a NullPointer TryCatch block because you think your code is so perfect and it's only the user that can provide null values. What a great idea!
  • 3
    @GiddyNaya > "But why would someone catch an exception if it could be prevented?"

    Exactly. Don't know if it's an age or experience 'thing'. Majority of devs in our web department are in their low/mid 20's and say, for example, if they call a function and it returns NULL (raising a null reference exception in their code), then it's a bug in someone else's code (not their responsibility to defend against bad/malicious behavior).

    Last weekend we had a DoS attack and somehow the data from the attack got to our SQL server (bad data throwing sql exceptions). My thought 'OMG...clean the bleeping data before you call the stored procedure!'. Their response to the attack 'We have blocked the IP. Won't happen again.'
  • 0
    @PaperTrail its almost as if you got the previous developer on the project I got to take over.

    Had I known that I would basically be editing 80-90% of the existing files...I would've just started over.
  • 0
    Why not return 0 on sales report data for that exception? Its not like you're writing code for a nuclear reactor or other real-time system.
  • 0
    At least comment it properly...
Add Comment