5 ms·
This is definitively wrong. Not to comment on "modern" python, but in general if this code returned an exception on division by zero it is definitively worse.
by deltaonefour 5y ago
This is definitively wrong. Not to comment on "modern" python, but in general if this code returned an exception on division by zero it is definitively worse.
The reason is because an exception is a runtime check. It caught an error that is hidden until runtime. The better way is to encode this logic into a static type checking system so this error isn't even permitted to run.
With the type Optional[Int], Any other function that utilizes the output type of this function MUST also be an Optional[Int] and not just an Int, meaning that the function signature must handle the None case and this is caught statically before run time. Insofar as the function type signature... python type checkers should handle this error.
However tbh I'm not sure how extensive type checkers for python are and whether or not they can track this type of error for the logic itself. For example while the below will catch a type error if you feed it a bad parameter, the signature itself could be wrong and not caught by a type checker:
x1: Int = 2 # results in type error (Good)
x2: Optional[Int] = None # results in runtime error. This is bad in terms of safety
def addOne(x: Optional[Int]) -> Optional[Int]:
return x + 1
As far as I know the above is permitted by python type checkers when truly correct code is below:
def addOne(x: Optional[Int]) -> Optional[Int]:
return x + 1 if x is not None else None
However with python 3.10 pattern matching the level of static type checking can increase to the point where this type of safety is 100% possible: https://www.python.org/dev/peps/pep-0636/ https://www.python.org/dev/peps/pep-0636/.
x1: Int = 2 # results in type error (Good)
x2: Optional[Int] = None # No error (also good)
def addOne(x: Optional[Int]) -> Optional[Int]:
match x:
case None: # if this case were deleted should result in static type error (also good)
return None
case _:
return x + 1
Not sure if python type checkers can handle the above code, but exhaustive type checking is 100% viable in the future if the syntax utilizes the pattern matching shown above. Haskell and Rust already have this level of type safety. See: https://rustc-dev-guide.rust-lang.org/pat-exhaustive-checking.html https://rustc-dev-guide.rust-lang.org/pat-exhaustive-checkin...
What this means in short is that you are wrong. Whatever your notions of "modern" python should be having division by zero return a runtime error instead of a None or an Error type is definitively worse. We are at a point in our technology that this type of error should be caught automatically BEFORE a program even runs. All type errors should be encoded into types and caught via static type checking and NOT via exceptions.
- globular-toast 5y agoThis is probably all correct, but man... this just isn't fun at all. This isn't why I got into programming.
- deltaonefour 5y agoI'm sorry you don't find programming fun. I find it fun. And if you don't like this stuff, maybe it's the wrong career choice for you.
- lkitching 5y agoFor division you really should put the requirement for the non-zero check in the type of the divisor instead of propagating failure to the caller e.g. div(x: Int, y: NonZero[Int]) -> Int Exceptions at least have the benefit of retaining the location the context for the error occurred which gets lost without extra bookeeping when using optionals.
- Spivak 5y agoThis ends up being a super unergonomic API in practice because you would need to modify the type-checker to infer positive integers correctly. Nobody wants to go through the effort of NonZero = NewType(‘NonZero’, int) if x != 0 x = NonZero(x) just to call your function, especially since you still have to handle the case where someone does NonZero(x) blindly without checking. Should the constructor throw?
- lkitching 5y agoThe check has to go somewhere and the caller to div has the most context in the event the divisor is 0. If you return an optional from div then you either impose the check on all the callers, or just propagate None everywhere and some top-level function has to deal with mysteriously missing values. The NonZero type should be responsible for checking the wrapped value is non-zero, you will probably want safe and unsafe constructor functions Int -> Optional[NonZero[Int]] Int -> NonZero[Int] where the unsafe version throws. If this is overkill for your application then I'd prefer throwing an exception in div rather than encoding the failure in the return type.