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

Random generation of CUSIPs, PPNs, and ISINs #234

Merged
merged 5 commits into from Apr 12, 2023
Merged

Random generation of CUSIPs, PPNs, and ISINs #234

merged 5 commits into from Apr 12, 2023

Conversation

ViridianForge
Copy link
Sponsor Contributor

@ViridianForge ViridianForge commented Apr 8, 2023

There are quite a few unique strings used in the financial industry to identify securities - three of which are:

  • Cusip -- 9 digit code that uniquely identifies North American securities
  • ISIN -- 12 digit code that uniquely identifies securities per ISO 6166

This PR adds means to randomly generate them, along with correctly calculated check digits. I primarily had in mind random generation of valid versions of these strings for folks using Golang in the securities trading space.

I also updated the benchmark file as far as I could when prepping this PR, as it appeared to be a bit out of date when I ran benchmarks for my code.

@brianvoe
Copy link
Owner

I think this pr looks really good! Nice job! Im going to go through this and add some comments and once they are tackled ill get it merged in.

finance.go Outdated Show resolved Hide resolved
@brianvoe
Copy link
Owner

I think the only other thing I want to see if you can try to simplify is the sub functions used. because gofakeit has so many functions I really try to only have the sub/private functions necessary to accomplish what that generated function is trying to accomplish. Because otherwise there becomes more and more private functions that people have to wrap there heads around. Im might be overthinking it but its something thats helped me keep gofakeit as simple to use and understand while reading the source code.

I see some of the functions might be necessary to extract out due to it being used in both Cusip and PpnCusip but if you can come up with a way of simplifying it so it has as least amount of sub function calls as possible that would be a huge help.

Let me know your thoughts. Thanks!

@ViridianForge
Copy link
Sponsor Contributor Author

Sure thing @brianvoe! I think it should be pretty doable to reduce PPN to a flag taken by the CUSIP function and roll from there. Really, PPNs are a weird thing that I only thought to include in the event someone actually needed them for their test code.

With the Check Digit subfunctions - I'll see if I can simplify those down - but I had initially wanted to keep them around so that generated CUSIPs/ISINs were valid when used in test cases.

@brianvoe
Copy link
Owner

Hey @ViridianForge just checking in. I wanted to get this merged in cause I am about to do a decent size release and wanted to get your pr in before I do. Let me know if you can tackle the minor updates we talked about.

Thanks.

@ViridianForge
Copy link
Sponsor Contributor Author

Yup! I should be able to push up later tonight if that's quick enough. I removed the PPN component as unneeded fluff and simplified things quite a bit.

Unfortunately, Comcast took my neighborhood down for "planned maintenance", so I am sitting a bit dead at the moment.

@brianvoe
Copy link
Owner

no problem. Ill give you another night and try to merge it in the morning. Thanks!

@ViridianForge
Copy link
Sponsor Contributor Author

@brianvoe - just pushed up those simplifications - though if you've got some ideas on how to simplify further, happy to do so.

@brianvoe brianvoe merged commit a5c348c into brianvoe:develop Apr 12, 2023
12 checks passed
@ViridianForge ViridianForge deleted the ViridianForge/finance_items branch April 13, 2023 04:53
@brianvoe
Copy link
Owner

https://github.com/brianvoe/gofakeit/releases/tag/v6.21.0

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