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

Remove redundancy in filenames #35

Open
AntoineRondelet opened this issue Nov 1, 2021 · 2 comments
Open

Remove redundancy in filenames #35

AntoineRondelet opened this issue Nov 1, 2021 · 2 comments

Comments

@AntoineRondelet
Copy link
Contributor

Currently, and across most our projects , we have lots of redundancy in our file names. In may cases, we have things like:
gadgetlib1//pairing/mnt/mnt_pairing_params.hpp. The mnt emphasized here: mnt/**mnt**_pairing_params.hpp is not necessary. We could simply have things like gadgetlib1//pairing/mnt/pairing_params.hpp, gadgetlib1//pairing/bw6_761_bls12_377/pairing_params.hpp etc etc. In that sense, we can talk about the pairing_params.hpp file from the given pairing group folder.
The example above highlight some of the redundancy we have in file names across the different code bases:

  • Zeth
  • Zecale
  • Libsnark/ff/fqfft

To action on this ticket, we need to potentially restructure the code, then rename the files, fix the #include directives and modify the #ifndef/define directives (with associated comment at the end of the files)

@dtebbs
Copy link
Contributor

dtebbs commented Nov 1, 2021

There may be a few more moving parts to this kind of change.

It's pretty common to name files to be consistent with the name of the (main) class they contain, and I think this kind of consistency (file name and class name) is significantly more important than removing a little redundancy in file names. Indeed, some languages actually enforce this kind of rule on class and file names.

We can remove the redundancy in the file names, but in that case I think we need to make better use of namespaces so that the file gadgetlib1/pairing/mnt/pairing_params.hpp contains the class libsnark::gadgetlib1::pairing::mnt::pairing_params, and not a class called mnt_pairing_params.

Also related to #29 (comment)

@AntoineRondelet
Copy link
Contributor Author

Yes this is a good point you're making here @dtebbs
I would be keen to think more about using namespaces in the various code bases. There is room for better code organization and separation here as we mostly use a single "holdall" namespace so far. I mean, the main namespaces are lib<name> and test as far as I remember. We use an internal namespace at one point in Zeth, but we generally do not have a very refined way of exposing the code in our cpp libraries. There may be an argument to change that down the road and use more layers of namespaces.

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

No branches or pull requests

2 participants