mithun218 / GiNaCDE

GiNaC Differential Equation Solver
MIT License
9 stars 1 forks source link

Encapsulate library functionality into classes and/or namespaces #11

Open peanutfun opened 2 years ago

peanutfun commented 2 years ago

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 commented 2 years ago

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