Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions demo_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ class BaseNumberGenerator:
def __init__(self):
self.limits = (1, 10)

def get_float(self, f):
raise NotImplemented
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raise NotImplemented causes TypeError at runtime


The code raises NotImplemented, which is a special singleton value, not an exception. This will cause a TypeError to be raised at runtime, which can be misleading and obscure the developer's intent.

Replace NotImplemented with NotImplementedError() to correctly signal that a method is abstract.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NotImplemented constant raised instead of NotImplementedError exception


The code raises NotImplemented, which is a special singleton value used in rich comparison methods, not an exception. Attempting to raise it will result in a TypeError at runtime.

To indicate that an interface method must be implemented by subclasses, NotImplementedError should be raised instead.


def get_number(self, min_max):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method doesn't use the class instance and could be converted into a static method


The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

raise NotImplemented
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raising NotImplementedType while only classes or instances are allowed


Raising objects other than a class, an instance or a string can cause a TypeError.

There can be cases where the value being raised is dynamically set. One common example is initially setting the value to None and assigning other objects based on a condition. Doing so can affect readability and is generally not recommended. In such cases, either re-write the block or ignore the issue for this file.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'raise NotImplemented' should be 'raise NotImplementedError'


While returning NotImplemented would be fine, raising it doesn't work and will cause a TypeError because NotImplemented is not an exception type.


Expand Down