skwasjer / IbanNet

C# .NET IBAN validator, parser, builder and generator
Apache License 2.0
119 stars 31 forks source link

Add a new IsQrIban property on ValidationResult for Switzerland and Liechtenstein #37

Closed 0xced closed 2 years ago

0xced commented 2 years ago

Pretty much everything is explained in the documentation of this new property.

codecov-commenter commented 2 years ago

Codecov Report

Merging #37 (dd82aea) into develop (c7c9b5b) will decrease coverage by 1.37%. The diff coverage is 85.63%.

Impacted Files Coverage Δ
...rc/IbanNet/Builders/BankAccountBuilderException.cs 100.00% <ø> (+75.00%) :arrow_up:
...t/CheckDigits/Calculators/InvalidTokenException.cs 100.00% <ø> (ø)
...ndencyInjection/IbanNetOptionsBuilderExtensions.cs 96.07% <ø> (-0.36%) :arrow_down:
src/IbanNet/IbanFormatException.cs 100.00% <ø> (ø)
src/IbanNet/Registry/BankStructure.cs 100.00% <ø> (ø)
src/IbanNet/Registry/BbanStructure.cs 100.00% <ø> (ø)
src/IbanNet/Registry/BranchStructure.cs 100.00% <ø> (ø)
src/IbanNet/Registry/IbanStructure.cs 100.00% <ø> (ø)
src/IbanNet/Registry/Patterns/PatternException.cs 100.00% <ø> (+25.00%) :arrow_up:
src/IbanNet/Registry/Patterns/PatternToken.cs 93.93% <ø> (-3.04%) :arrow_down:
... and 55 more
skwasjer commented 2 years ago

Thanks for contrib, looks good. API wise though, we should not want to depend on ValidationResult. I am more inclined to add this as an extension method to the Iban type for few reasons:

0xced commented 2 years ago

You're absolutely right. Implementing this new IsQrIban property on Iban instead of ValidationResult made both implementation of the property and of the tests simpler so that's definitely a win! Also, the coverage should not drop by -37.50% as it did in the previous implementation.

I have rebased my qr-iban branch and force-pushed the new implementation.

skwasjer commented 2 years ago

Thanks, merging, I may change it to an extension method though before releasing.