Refactored complex methods #263
Conversation
|
Thanks! I should have time to review this tomorrow. |
vladimirnani
force-pushed
the
refactor
branch
from
0fa5cbd to
998cac1
Compare
|
|
||
| mccabe: | ||
| options: | ||
| max-complexity: 9 |
There was a problem hiding this comment.
There should be a new line here.
| if len(eik) == 13 and not check_eik_extra(eik): | ||
| raise ValidationError(error_message) | ||
| eik_validator = EIKValidator() | ||
| egn_validator = EGNValidator() |
There was a problem hiding this comment.
What do you think about deprecating the *_validator versions? I suppose there's no real harm in keeping them around but it would be nice to provide just the class version. Users can always make their own version of the callable if they want to use it that way. Consider this optional and only do it if you agree.
There was a problem hiding this comment.
Yes I was thinking to deprecate them. There is no point in callable, because usually logic is complex so you have to split into several methods. And even for simple cases think is better to stick to class validators conventionally.
| """ | ||
| Check Bulgarian unique citizenship number (EGN) for validity. | ||
|
|
||
| More details https://en.wikipedia.org/wiki/Unique_citizenship_number | ||
| Full information in Bulgarian about algorithm is available here | ||
| http://www.grao.bg/esgraon.html#section2 | ||
| """ | ||
| def check_checksum(egn): | ||
|
|
||
| def check_checksum(self, egn): |
There was a problem hiding this comment.
Can you mark new methods as private (prefixed with _)? This applies to all of the methods that you've created on the forms. I want to be clear that these are internal and not public. Please to this for data fields on the form as well.
benkonrath
changed the title
vladimirnani
force-pushed
the
refactor
branch
from
998cac1 to
9c35233
Compare
|
Thanks for review, have created 265. |
- Complexity 9 - Moved function validators to Validator classes - Changed Indonesian Numbers
vladimirnani
force-pushed
the
refactor
branch
from
9c35233 to
910333d
Compare
|
Thanks @vladimirnani! |
Specified complexity level for prospector.
Refactored methods that raised complexity exceptions. (max-complexity 9)
Fixes #261