Skip to content

Conversation

@lisotton
Copy link

@lisotton lisotton commented Nov 5, 2025

I think it will be a great addition to add a method to extract a helpful text from the formatter, so if the library is used in some application to validate the postcode, the hint can be used to provide why the provided postcode is invalid.

@lisotton lisotton force-pushed the add-hint-method branch 2 times, most recently from aeaf601 to 79bf981 Compare November 5, 2025 10:27
Copy link
Member

@BenMorel BenMorel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @lisotton, this is an excellent suggestion, thank you for taking the time to implement it!

Something that caught my eye is that CountryPostcodeFormatter::hint() is a fixed message that is only ever used to enrich the exception after we detect that a postcode is not right (and therefore have more information about the violation). Why not make CountryPostcodeFormatter::format() throw the exception, adding a custom hint based on the error?

This would fix elegantly I think, what I pointed out on the AF formatter: depending on the input, we could have either of these errors:

  • Postcodes consist of 4 digits, without separator.

  • Province code (first 2 digits) must be between 10 and 43.

What do you think?

{
public function hint(): string
{
return 'Postcodes consist of 4 digits, without separator.';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be really useful, we would need to mention the 10-43 province code as well.

@lisotton
Copy link
Author

lisotton commented Nov 7, 2025

@BenMorel I totally agree with your suggestion, but it took me already A LOT OF TIME to go through all the 182 classes, if we to go each class and each condition, it will be muuuuuch more time.

@BenMorel
Copy link
Member

I understand. I may find some time to help, if we can split the work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants