Ranter
Join devRant
Do all the things like
++ or -- rants, post your own rants, comment on others' rants and build your customized dev avatar
Sign Up
Pipeless API
From the creators of devRant, Pipeless lets you power real-time personalized recommendations and activity feeds using a simple API
Learn More
Comments
-
Kudos to the team finding this and on you recognizing the mistake. This was a very important lesson my friend! thanks for sharing with us!
-
C0D4681385yand this people, is why you having staging environments.
Manual testing and even unit testing would pick this up before you got that to production. -
SomeNone7135yBurned by C#'s rudimentary type system I guess… and while I realize that this is an existing codebase, and can probably not be refactored to use a stronger domain model, I dare inject here that using a stronger typed language such as F# would have allowed to produce a domain model where this mistake would have been quite impossible to make. Google 'Domain Modeling Made Functional' if you are interested.
-
rantbook7355yWhy would you publish this in production if it wasn't her/him? Why wouldn't you *test* such job-threatening system-critical code?
-
SomeNone7135y@hatemyjob Why would using ``x == false`` instead of ``!x`` make any difference at all?
-
C0D4681385y@SomeNone logically, no difference.
Legibility though, it's harder to miss read == / === over !x -
This should have been tested through unit tests lol
These things can be missed with peer reviews as well :( -
@SomeNone slight bump in readability.
“==false” has less chances of being mistaken with “x”
than
“!x” instead of “x”
Functionally the same. The likelihood of skimming and missing this bug is far far less if you use the method no 1. -
For reasons like this I prefer to write:
if( a == false)
If OP don't mind me asking, why ResponseStatusCode is inside Constants class and not an enum on its own? like HttpStatusCode? -
@gitpush ResponseStatusCode is an enum inside Constants class which holds other types of status code and it's also socket related not http. Would find other ways of improving it.
-
How could a critical bug like that make it's way past your own QA as you're developing? Did you just write the code, hit compile and pour yourself a beer?? You guys don't have any environments before production?
-
You should have a mapper (a simple dictionary) to automatically pass the bool and get the value.. I would feel more safe with that
-
@Hastouki I don't know about them but we have test environments but do not write unit tests and do joy have QA testers. So...this seems soooooo typical to me.
:'(
Related Rants
-
ChargerIIC7*Doing a Peer Code Review of someone senior to me* Me: This fix doesn't look like it will work, but maybe I d...
-
projektaquarius3Me: Are you sure you want this in the acceptance test procedure? Lead: Yes. Me: I'm just saying, we don't hav...
-
xorith1The worst thing I've seen another developer do is not give constructive criticism where needed, as well was fa...
Peer review is a life saver!!! My colleague just saved me my job as i almost published this fucking block to production.
rant
almost slipped
fucking not-symbol
peer review