And for every one you notice, you miss three. Because the system is too complex. This is why we need to program defensively.
It was used before, once every startup of the application so it was deemed OK.
And that's where someone went wrong. There should have been a test, or runtime failure, or even better (but more difficult) a compilation failure if someone misuses something in that way.
> And that's where someone went wrong. There should have been a test, or runtime failure, or even better (but more difficult) a compilation failure if someone misuses something in that way.
I don't subscribe to the idea of creating so much additional code for a relatively simple constraint.
Code should be dense. Dense means a lot of important constraints implemented in small amount of code.
Of course this is not yet guarantee that the code is readable and easy to maintain, but as a general rule, a codebase that has 5 times the amount of code to implement same functionality will more likely than not be less maintainable. That is because it is just more difficult to work with that much code.
> There should have been (...) or even better (but more difficult) a compilation failure if someone misuses (...)
In this particular case what I suggested was to move the original function with generic name from a generic utility package to a specific module package with specific name.
See? Compilation failure, not difficult at all.
Code should be dense. Dense means a lot of important constraints implemented in small amount of code.
Sorry, but I can't believe how wrong this is. "Dense" is a really poor metric for code quality. "Simple" is a much better metric. And one aspect of "simple" is "not interwoven".
So the question here is, could the new code I'm advocating be implemented in a way that is simple and not interwoven? That is, not adding complexity to any of the rest of the code base. It is nothing more than a safer leaf.
And the answer is, in this case, yes. This does not increase the complexity of the code base. The amount of code added is commensurate with the need to make sure it's not a foot gun.
Clever naming, or visibility restriction, is a poor solution. You may prevent the function from being called explicitly more than once. But you don't prevent future refactors from leading to the function that calls that thing being called more than once.
The crucial bit of humility that is necessary is to realize that there's no way you can foresee how these problems will come up in the future. The only thing you can do is be defensive.
I agree wholeheartedly and I wish more developers felt this way.
I hate opening new projects and seeing the potential for null reference exceptions everywhere when you could instead just check the constraints directly and give a more meaningful response.