Shimul-Baidya / MBDyn

GNU General Public License v2.0
1 stars 1 forks source link

Compare enum values using members, not strings #6

Open Maarrk opened 2 days ago

Maarrk commented 2 days ago

It would be better to use enum members (like LawType.SCALAR_ISOTROPIC_LAW) with the equality operator rather than strings (like "scalar isotropic law"), so you can't introduce an error by copying the text wrong - if you make a typo in the member name, type checker will highlight that.

A good rule of thumb is if you use an enum, there is only one place in the code where you see the string or number it corresponds to, and everywhere else you refer to it.

https://github.com/Shimul-Baidya/MBDyn/blob/80f359c36febf36195c5d767f7db161aa880a14e/contrib/PythonPreprocessor/MBDynLib.py#L3816-L3827


Other notes: it's good that you check the else case and raise an informative exception.

Since you marked dim() with @property, I think you can use it like {self.dim} here https://github.com/Shimul-Baidya/MBDyn/blob/80f359c36febf36195c5d767f7db161aa880a14e/contrib/PythonPreprocessor/MBDynLib.py#L3833

Maarrk commented 2 days ago

To make this more enforced by the type checker and easier to follow where it should be enum and where string, we could remove str from LawType base classes and use .value where we need it as a string.