Fixed an off by 1 issue related to maxmium GeneratorId.#62
Fixed an off by 1 issue related to maxmium GeneratorId.#62RobThree merged 2 commits intoRobThree:masterfrom Throwy:master
Conversation
Max generator id was being calculated as 1 higher than the actual max. This has been corrected and the message in the exception thrown when the given generator id is invalid has been appropriately re-worded to include 0 and maxGeneratorId.
|
Oh, wow, I can't believe I missed this. Would you be so kind to also include a unittest that tests for the max and max+1 cases (using an |
|
I sure can. I'll work on updating the PR today. Funnily enough, I only found this because the wording 'between' was slightly misleading and was just going to make a PR to change the wording. |
IdGenTests/IdGeneratorTests.cs
Outdated
| public void Constructor_Throws_OnInvalidGeneratorId_Positive_MaxPlusOne() | ||
| { | ||
| var structure = new IdStructure(41, 10, 12); | ||
| var maxgeneratorid = GetMaxGeneratorId(structure); |
There was a problem hiding this comment.
Just a little nitpick: I'd hardcode this to the actual value; if there's a bug in the ("copied" or "lifted") GetMaxGeneratorId code then the bug will also be in this test. So I guess I'm saying is I'd set maxgeneratorid to 1023 here and maybe even add a little comment making clear it's related to the 10 in the IdStructure.
But, again, thanks for the PR. I can make this (tiny) change myself later when I get back if you want to.
There was a problem hiding this comment.
That's fair. I updated the PR for that
|
I just published a new release. Thanks for your work! Much appreciated 👏 👊 |
|
Thanks and no problem! I'm excited to start using this |
2 participants
Max generator id was being calculated as 1 higher than the actual max. This has been corrected and the message in the exception thrown when the given generator id is invalid has been appropriately re-worded to include 0 and
maxgeneratorid.