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

Encapsulate library functionality into classes and/or namespaces #11

Open
peanutfun opened this issue Nov 10, 2021 · 1 comment
Open

Comments

@peanutfun
Copy link

Description

GiNaCDE defines variables controlling its behavior in its header files. After including the headers, these variables are publicly available. At first, this is confusing because variables can be set that have never been explicitly defined in the user code. But there is the even greater problem of unintended variable shadowing:

#include <GiNaCDE/GiNaCDE.h>

int main()
{
  output = maple;  // Fine, setting the output for GiNaCDE
  int output = mathematica;  // Shadowed variable, GCC 11 does not issue a warning!

  // Now we lost all access to the GiNaCDE output...
  output = maple;  // Sets local `output`, not the one of GiNaCDE
}

One way of making such errors less likely is creating a separate namespace for GiNaCDE, so that the user code becomes more explicit:

#include <GiNaCDE/GiNaCDE.h>

int main()
{
  GiNaCDE::output = GiNaCDE::maple;  // Fine, setting the output for GiNaCDE
  int output = GiNaCDE::mathematica;  // Setting a local variable without shadowing
}

However, the problem remains that seemingly unrelated variables in the headers are changing the global behavior of the GiNaCDE library:

#include <GiNaCDE/GiNaCDE.h>

void do_something_else()
{
  // ...
  output = mathematica;
}

int main()
{
  output = maple;  // Fine, setting the output for GiNaCDE
  do_something_else();  // Accidentally changed the global output
}

Proposal

I think the variables and commands for controlling the behavior of GiNaCDE could be encapsulated into one or several classes. This would make the public interface more intuitive and avoid unintentional resets or shadowing of interface variables. One (incomplete!) example would be

// In <GiNaCDE.h>
class GiNaCDESolver
{
public:
  // Behavior variables
  int output = maple;
  bool positivePart = true;
  bool negativePart = false;
  std::string filename = "output.txt";

  // Interface functions
  int desolve(/*...*/);

private:
  // Internals...
}

// In user code executable
#include <GiNaCDE/GiNaCDE.h>

int main()
{
  GiNaCDESolver my_solver;
  my_solver.output = mathematica;
  my_solver.negativePart = true;

  GiNaCDESolver my_other_solver;
  my_other_solver.output = maple;
  my_other_solver.filename = "stuff.txt";

  my_solver.desolve(/*...*/);  // Writes mathematica output into "output.txt"
  my_other_solver.desolve(/*...*/);  // Writes maple output into "stuff.txt"
}

Related issues

openjournals/joss-reviews#3885

@mithun218
Copy link
Owner

@peanutfun Thanks a lot for your great suggestion! I will work soon on your proposal.

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

No branches or pull requests

2 participants