-
Notifications
You must be signed in to change notification settings - Fork 21
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 underscore from module names #7
Comments
Curious as to why this is the case too… |
While we are at it we can try something like the structure indicated in this wiki article. Here is the proposed restructuring based on nature of the distribution's support, since this library effectively works with discrete distributions. Distribution
|_Finite
| |_Uniform
| | |_GSL
| | |_Java
| | |_Ruby
| |_ ...
|_Infinite
|_Normal
| |_GSL
| |_Java
| |_Ruby
|_Bernoulli
|_ ... let me know what you think |
@vaibhav-y that might work, but I would prefer a more shallow structure, like Of course, of the current issues this is the least important, so if we don't know a clearly better structure, let's not change it (yet). We should provide good arguments for changing something like this. :) |
I gave it another thought later and what we have right now is quite good. I was toying with the idea of a factory pattern being used to create distribution objects, and this seems to fit the use case of the pattern itself. Something like this calls a factory constructor (which will work quite well with the current structure) vaibhav-y/statistical illustrates the idea I have in mind when I say factory pattern for distribution & rngs. |
Each distribution under
Distribution
has several different implementations depending on the libraries installed: pure Ruby, GSL, Java or statistics2.Each implementation lives under a module named after the library with an underscore, e.g.
Ruby_
,Java_
.Is this underscore necessary? Can we find a better name for these modules? Or can we restructure the distribution gem so that these modules aren't necessary anymore?
The text was updated successfully, but these errors were encountered: