-
-
Notifications
You must be signed in to change notification settings - Fork 268
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
Random generation of CUSIPs, PPNs, and ISINs #234
Conversation
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. |
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! |
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. |
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. |
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. |
no problem. Ill give you another night and try to merge it in the morning. Thanks! |
@brianvoe - just pushed up those simplifications - though if you've got some ideas on how to simplify further, happy to do so. |
There are quite a few unique strings used in the financial industry to identify securities - three of which are:
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.