Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add includeFormatSymbols for cnpj & cpf in brazil extensions #269

Merged
merged 4 commits into from Nov 30, 2019
Merged

Add includeFormatSymbols for cnpj & cpf in brazil extensions #269

merged 4 commits into from Nov 30, 2019

Conversation

ArthNRick
Copy link
Contributor

Often developers need this data (cpf and cnpj) to be numeric, unformatted, that's what this implementation does.

@bchavez bchavez changed the title Create numeric cnpj & numeric cpf in brazil extensions Add includeFormatSymbols for cnpj & cpf in brazil extensions Nov 30, 2019
@bchavez bchavez merged commit ecdb311 into bchavez:master Nov 30, 2019
@bchavez
Copy link
Owner

bchavez commented Nov 30, 2019

Hi Arthur,

Thank you for the Pull Request.

I refactored the code to include an includeFormatSymbols parameter in Cnpj() and Cpf(). When includeFormatSymbols = false the methods don't return formatted symbols.

For example, company.Cnpj(includeFormatSymbols: false) returns "61860606000191" (string).

The reasoning to keep it as a string is mostly because as a string, it can be converted to long or ulong or be kept as an unformatted string.

In the initial PR, NumericCpf() returned ulong which is somewhat more restrictive to consumers who might want long or unformatted string.

This means however if you want to consume Cnpj(includeFormatSymbols: false) or Cpf(includeFormatSymbols: false) method, you'll need to ulong.Parse() for your own purposes.

Bogus v28.4.2 is now available with these changes:
https://www.nuget.org/packages/Bogus/28.4.2

void Main()
{
   var c = new Company();
   
   var cnpjList = Enumerable.Range(1,10)
                  .Select(_ => c.CnpjNumeric())
                  .ToArray();
   cnpjList.Dump();
}

public static class MyCustomExtensionsForBogus
{
   public static ulong CnpjNumeric(this Company c){
      return ulong.Parse(c.Cnpj(includeFormatSymbols: false));
   }
}

LINQPad_3614

I hope that helps.

Thanks,
Brian

@ArthNRick
Copy link
Contributor Author

The only problem with this solution is the context, when a value is requested with formatting, it will be impossible to get the same value without formatting, and vice versa. In separate methods, the context would eventually return the same value, and the method would replace.

@bchavez
Copy link
Owner

bchavez commented Dec 2, 2019

Is Cpf the only one with Person context? Did context work in previous version?

@ArthNRick
Copy link
Contributor Author

Is Cpf the only one with Person context? Did context work in previous version?

The CPF is for the Brazilian the same as the SSN is for the Americans, so the same person can not have more than one number, it is unique for each individual. Therefore it must be kept in context to prevent data from being randomized. The new version still works, but once I generate formatted data, I can no longer get it without formatting, since it is in context with the first value

@bchavez
Copy link
Owner

bchavez commented Dec 3, 2019 via email

@ArthNRick
Copy link
Contributor Author

ArthNRick commented Dec 3, 2019

Both the current and previous versions returned the same value by exiting the following code:

var p = new Person ();
var isEquals = p.Cpf () == p.Cpf () // true;

This is correct and there is no problem.

but with the latest version, p.Cpf () == p.Cpf (false); would return true as well, but should be false since p.Cpf (false) would not be formatting, and p.Cpf() formatted. I will create a new PR with a possible solution, and then you can evaluate if it is ideal

@bchavez
Copy link
Owner

bchavez commented Dec 4, 2019

Ah, thank you @ArthNRick. I understand the issue now. We'll get that fixed.

bchavez added a commit that referenced this pull request Dec 4, 2019
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.

None yet

2 participants