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

Naming convention for classes and functions containing acronyms #1092

Closed
jaeandersson opened this issue Apr 15, 2014 · 8 comments
Closed

Naming convention for classes and functions containing acronyms #1092

jaeandersson opened this issue Apr 15, 2014 · 8 comments
Assignees
Labels
Milestone

Comments

@jaeandersson
Copy link
Member

I propose to treat all acronyms in class and function names as any other word and not capitalize them like now. This would mean e.g.:

  • QPSolver becomes QpSolver
  • NLPSolver becomes NlpSolver

NOTE: The rest of this issue has become moot due to #786.

  • QCQPQPSolver becomes QcqpQpSolver
  • SOCPQCQPSolver becomes SocpQcqpSolver
  • SCPgen becomes Scpgen
  • SQPMethod becomes SqpMethod
  • CSparse becomes Csparse
  • QPOasesSolver becomes QpoasesSolver (or Qpoases, see below)
  • CVodesIntegrator becomes CvodesIntegrator (or Cvodes, see below)

Secondly, I propose the to drop the Solver and Integrator suffices for interfaced tools:

  • Ipopt instead of IpoptSolver
  • Qpoases instead of QPOasesSolver
  • Knitro instead of KnitroSolver
  • Idas instead of IdasIntegrator
  • Cvodes instead of CVodesIntegrator
  • Snopt instead of SnoptSolver
  • CollocationIntegrator keeps current name (not an interfaced tool)
@jaeandersson jaeandersson added this to the Version 2.0.0 milestone Apr 15, 2014
@jaeandersson
Copy link
Member Author

Note that there is currently no convention imposed at all. Acronyms are sometimes upper-case, sometimes not (e.g. IpoptSolver, CSparse, nlpSolverIn, daeOut).

@jgillis
Copy link
Member

jgillis commented Apr 15, 2014

Camel case sounds fine.
Dropping suffixes, I'm not so conviced of that.
It is cleaner to read "IdasIntegrator" than "Idas" if you are not familiar with the sundials suite.
Ahso, there may be external packages that expose multiple functionalities, such as GSL, SLICOT

@jaeandersson
Copy link
Member Author

Dropping suffixes, I'm not so conviced of that.
It is cleaner to read "IdasIntegrator" than "Idas" if you are not familiar with the sundials suite.
Ahso, there may be external packages that expose multiple functionalities, such as GSL, SLICOT

I do not agree on this. I think Idas, Ipopt reads better than IdasIntegrator, IpoptSolver. For multiple functionalities, not having suffix is preferably IMO. Having GslFoo and GslBar looks much better to me than either GslIntegratorFoo/GslIntegratorBar or GslFooIntegrator/GslBarIntegrator.

We could also use that convention for Sundials and call the integrators SundialsIdas and SundialsCvodes.

@jaeandersson jaeandersson self-assigned this Jul 9, 2014
@jaeandersson
Copy link
Member Author

This issue becomes much easier (moot?) after #786.

@jaeandersson
Copy link
Member Author

@jgillis @ghorn After #786, this issue basically boils down to whether we want to change:

  • NLPSolver to NlpSolver
  • QPSolver to QpSolver
  • LPSolver to LpSolver
  • SDPSolver to SdpSolver
  • SDQPSolver to SdqpSolver
  • QCQPSolver to QcqpSolver
  • SOCPSolver to SocpSolver

If not, what do we do with DpleSolver?

Please comment.

@ghorn
Copy link
Member

ghorn commented Jul 16, 2014

I like NlpSolver over NLPSolver

@ghorn
Copy link
Member

ghorn commented Jul 16, 2014

... and all the other ones QpSolver, LpSolver, etc

@jaeandersson
Copy link
Member Author

Yeah, me too. I like following a convention.

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

No branches or pull requests

3 participants