Add non-panicking MaLo ID checksum function #279
Conversation
| errs := make([]error, 0) | ||
| if first := maloID[0] - '0'; first < 1 || first > 9 { | ||
| errs = append(errs, fmt.Errorf("first char must be a digit from 1-9, is '%c'", maloID[0])) | ||
| } | ||
|
|
||
| evenSum, oddSum := 0, 0 | ||
|
|
||
| for index, digitRune := range maloID[:10] { | ||
| digit := int(digitRune - '0') | ||
| if digit < 0 || digit > 9 { | ||
| errs = append(errs, fmt.Errorf("char at index %d must be a digit from 0-9, is '%c'", index, digitRune)) | ||
| } | ||
|
|
||
| // Note: The specification for the MaLo defines the position such that the first digit has the position 1, | ||
| // unlike indexing in Go, which starts at 0. To avoid confusion, we explicitly define the position here. | ||
| position := index + 1 | ||
|
|
||
| if position%2 == 0 { | ||
| evenSum += digit | ||
| } else { | ||
| oddSum += digit | ||
| } | ||
| } |
There was a problem hiding this comment.
I found the old approach a bit easier to read. instead for checking each and every character on their own, whether it's a digit, a simple match check with [1-9]\d{9,10} seems more straigh forward to me. Doesn't one fail early in go?
There was a problem hiding this comment.
It's a trade-off. A regex check would have been much shorter on the implementation side, but it makes for less readable error messages on the client side. Although in this case the regex is pretty simple and also it would be possible to translate to some error message like "need a string that starts with a non-zero digit, followed by 9 or 10 more digits, got ".
I wouldn't see this as a very important issue as the exact validation is an implementation detail and could be changed later.
| @@ -112,6 +116,48 @@ func GetMaLoIdCheckSum(maloIdWithoutCheckSum string) int { | |||
| return (10 - (stepC % 10)) % 10 | |||
| } | |||
|
|
|||
| // CalculateMaLoIdCheckSum calculates the checksum (last digit) for the given malo ID. Takes both the 10-digit part of a MaLo or the whole MaLo (11 digits). | |||
| // Returns an error if maloID's length is neither 10 nor 11, or if the first ten characters do not constitute a maloID. | |||
| func CalculateMaLoIdCheckSum(maloID string) (int, error) { | |||
There was a problem hiding this comment.
I'm unsure if I'd prefer to return a rune or an integer.
This adds a new function,
CalculateMaLoIdCheckSum, that calculates the checksum for a given MaLoID or the first 10 digits of a MaLoID. The existing function,GetMaLoIdCheckSum, is marked as deprecated, but kept for backwards-compatibility.