Add "clean" option to be able to avoid preg_replace if you need

[ortegafernando]

Hi, I want to use this great library to test TAX numbers, but I dont want to use preg_replace, because, for example, it cleans "."

With this PR user is able to choose if they want to check the real string (it is only uppered case) or he/she wants to "clean it".

See this function in my PR vs original code.

tin/src/CountryHandler/CountryHandler.php

Line 34 in 39e5f92

public function getTIN(): string

https://github.com/ortegafernando/tin/blob/4a3cce4fa8e129385840ccf25b9d17aab6d88ccb/src/CountryHandler/CountryHandler.php#L36

Only this code added to this function:

if (!$this->clean) {
     return strtoupper($this->tin);
}

I think it doesn't hurt and can solve my and other's problem.

Regards,

[what-the-diff]

PR Summary

• Enhancement in the CountryHandler class
  A new trait '$clean' has been embedded into the CountryHandler class to improve its functionality. The constructor of this class has been updated to adopt this new property. Moreover, the existing method getTIN has been further improved to analyze the '$clean' value, and it will now return the input string in uppercase format if '$clean' is set to false.

• Improvement in the TIN class
  Similar to the CountryHandler class changes, an additional property '$clean' has been introduced to the TIN class. This update to the constructor will further enhance its functionality. The methods from and fromSlug have been updated to receive the '$clean' trait and transfer it to the constructor. The getAlgorithm process has been upgraded to deliver the '$clean' property to the handler constructor.

[drupol]

Thank you for your PR, I implemented it in a different way, let me know if this is fixing your issue.

[drupol]

Just FYI, it's been 3 years that this library has been set up by Thomas and me.

After 3 years, I believe that using the method getTin() is a bad design decision. Nobody should ever need to use it, since the caller own the original TIN string before calling this library.

I will take that in account into the next major release.

[ortegafernando]

Sorry, I have not explained well.

I am not using getTin function, I am using the normal check() function

What I have detected is that I want my check WITHOUT normalize Tin string.

I think the commit you have added still normalize the Tin string before checking patterns, and so on.

Could you add some kind of variable to be able to decide If i want the checking without normalizing it ?

May be I have explained now better.

[drupol]

Indeed, I don't understand your use case.

You can submit any type of Tin value to the library, its sole purpose is to validate it... and I believe that it is what it does now. Why would this be problematic in your use case? Can you please further develop?

[ortegafernando]

Thanks for your question. I am using this library to check TIN before the end user finally execute/submit info (including TIN) to an external service.
That external service doesn't like ".", spaces, ....

So I want to be more restrictive and I need to check TIN without normalizing it to warm user that he/she can not go on submitting the form and must check and correct the TIN number. (I don't want to do the error check by my own, I want user to check and correct that in order to learn for future occasions. Besides, the TIN is updated with the information submittion and I don't want to send and update a TIN number without the knowledge of the user. I prefer him/her to check and rewrite it by herself)

Thanks.

[drupol]

OK I got your point.

Thanks for your question. I am using this library to check TIN before the end user finally execute/submit info (including TIN) to an external service. That external service doesn't like ".", spaces, ....

Then it's your responsibility to clean the TIN, not to this library to do it.

So I want to be more restrictive and I need to check TIN without normalizing it to warm user that he/she can not go on submitting the form and must check and correct the TIN number. (I don't want to do the error check by my own, I want user to check and correct that in order to learn for future occasions. Besides, the TIN is updated with the information submittion and I don't want to send and update a TIN number without the knowledge of the user. I prefer him/her to check and rewrite it by herself)

Indeed, I'm not going to implement a normalization method publicly now, at least, not in this version, it's too touchy. Maybe this is something to keep in mind for the next version.

Cheers!

[ortegafernando]

@drupol Yes, it is my responsibility to "clean", but why I can not check the TIN string as I am sending it to your library? without any "internal modification of the library by itself"?

I am not asking to public the normalization method, only to be able to enable or disable it when I check a TIN string.

With a "clean" or "normalize" variable, we could choose if we want or not want this type of library's internal cleaning system.

It doesn't hurt to anyone, and it can help user cases like mine.

I dont know if you get my point. Thanks.

[drupol]

@drupol Yes, it is my responsibility to "clean", but why I can not check the TIN string as I am sending it to your library? without any "internal modification of the library by itself"?

Just out of curiosity, what's the format of the TIN you're sending in this library? Could you give me a valid use case why you would want to change the current state of things? I still do not fully get it.

I am not asking to public the normalization method, only to be able to enable or disable it when I check a TIN string.

This normalization will be done by this library, internally, and will not be exposed publicly. At least in this version (1) of the library.

With a "clean" or "normalize" variable, we could choose if we want or not want this type of library's internal cleaning system.

No, the job of this library is:

1. Take a string parameter, the TIN value
2. Validate it and return true or false
3. Nothing else

It doesn't hurt to anyone, and it can help user cases like mine.

Sure, it doesn't hurt. But I prefer to restrict the scope of the library to only validate TIN properly, and nothing else.

Thanks.

[ortegafernando]

For example, this one: 7888.8888S

Library's check is valid, but the TIN is not good for my next step (external goverment process of the data, they dont accept it). And as you see, the TIN is not good as it is written

About normalization, yes, i don't want you to exposed it, but I want to be avoided.

I agree with the main job of this library but I think there is something missing in the steps you have written (please don't take this as an ofense, it is only how to interpretate the code):

1. Take a string parameter, the TIN value
2. Normalize it
3. Validate it and return true or false
4. Nothing else

Or another way of writting it:

1. Take a string parameter, the TIN value
2. Validate "a normalized version of it" and return true or false
3. Nothing else

Do you understant me?, in fact, the library is doing something more, it is doing an internal normalization that... in my case (or many others) is not good. The library is not "just validating a TIN string", it is validating a "new version of it", a normalized version of it.

I hope I can explain what I mean.

[drupol]

(please don't take this as an ofense, it is only how to interpretate the code)

No worries :) I'm used to all these technical back and forth, and there is absolutely no offense in here. Hope it's the same for you.

Now I understand what you're saying and that make sense, you're right. So sad you didn't explain like that in your first message. I will see what I can do about that.

[drupol]

I reverted my previous commits and added a strict parameter. Let me know if that sounds better for you.

[ortegafernando]

Thanks a lot for this stict parameter, I think it can work, but I have a doubt.

'tin' => true === $strict ? $this->normalizeTin((string) $tin) : (string) $tin

If strict is true ..... then you normalize? is it not the other way round?

'tin' => true === $strict ? (string) $tin : $this->normalizeTin((string) $tin)

May be I am misunderstanding.

[drupol]

#brainfart Once again, you're right. I just fixed it!

[ortegafernando]

It seems very good now, I will update with composer requiere and check it. I think it is perfect. Thanks a lot. (do you want me to make a pull request about readme.md to update the usage?)

[drupol]

Yes please! That would be very appreciated :)