In working on a review for Solve a set of "restricted" linear equations efficiently, I decided to reimplement from scratch using the method I proposed in my answer.
The application
I won't repeat the entire specification here, but in a nutshell, this program is intended to solve a very restricted system of linear equations. In particular, each equation is specified to be of the following format:
var = var|value [+ var|value]*
Each var
is composed of one or more letters (only) and each value
is composed of digits (only) and fit into unsigned int
type (note that this differs slightly from the original, which made the assumption of unsigned long
but this does not pose a material difference here).
So, the equation y = x + 3
or foo = bar + baz
are acceptable, but y = x - 3
ory = x - 7
or bar + baz = foo
are not.
The program is to solve all linear equations and report the values for each variable in lexicographic order.
Further, the input is guaranteed to be correctly formatted and not to be cyclic or unsolvable.
My questions
I'm interested in ideas about class design and implementation, but not in error checking of the input (because it's guaranteed to be correctly formatted and not cyclic or unsolvable).
In particular, here are some possible nits:
- there's not much input error checking (yes, I know; see above)
- the value of the variable could overflow and wrap
- the name or value classifier only looks at the first character
Funny business
Originally, I was confused because the compiler I'm using (g++ (gcc) version 6.3.1) reported a peculiar warning that puzzled me, although the program compiles and runs correctly.
Specifically, I had to compile the code with the -fpermissive
flag, as per the following warning message:
../src/alt.cpp: In function ‘std::set<Variable, std::less<void> > solve(std::istream&)’:
../src/alt.cpp:98:51: warning: passing ‘const Variable’ as ‘this’ argument discards qualifiers [-fpermissive]
unsolved |= eq.replaceKnowns(equations);
^
../src/alt.cpp:63:10: note: in call to ‘bool Variable::replaceKnowns(std::set<Variable, std::less<void> >&)’
bool replaceKnowns(std::set<Variable, std::less<>> &equations) {
^~~~~~~~~~~~~
The reason is that all iterators for std::set
are const
, (hat tip to @Incomputable) so to accomodate, I've made the dependencies
and value
member data items mutable
and made the replaceKnowns
function const
. So the question now is: clever move or horrible hack?
The code
#include <iostream>
#include <set>
#include <vector>
#include <string>
#include <sstream>
#include <cctype>
#include <functional>
class Variable {
public:
friend std::ostream &operator<<(std::ostream &out, const Variable &var) {
out << var.name << " = ";
for (const auto &vname: var.dependencies) {
out << vname << " + ";
}
return out << var.value;
}
friend std::istream &operator>>(std::istream &in, Variable &var) {
Variable v{};
std::swap(var, v);
std::string line;
if (std::getline(in, line)) {
std::stringstream buff{line};
std::string token;
enum states {varname, equals, depOrConst, plus, error} state = varname;
while (buff >> token) {
switch (state) {
case varname:
var.name = token;
state = equals;
break;
case equals:
state = (token == "=") ? depOrConst : error;
break;
case depOrConst:
if (std::isalpha(token[0])) {
var.dependencies.push_back(token);
state = plus;
} else if (std::isdigit(token[0])) {
var.value += std::stoul(token);
state = plus;
} else {
state = error;
}
break;
case plus:
state = (token == "+") ? depOrConst : error;
break;
default: // error
in.setstate(std::ios::failbit);
return in;
}
}
}
return in;
}
bool operator<(const Variable &other) const {
return name < other.name;
}
bool unsolved() const {
return !dependencies.empty();
}
bool replaceKnowns(std::set<Variable, std::less<>> &equations) const {
std::vector<std::string> unresolved;
for (auto dep = dependencies.begin(); dep != dependencies.end(); ++dep) {
auto m = equations.find(*dep);
if (m != equations.end() && !m->unsolved()) {
value += m->value;
} else {
unresolved.push_back(*dep);
}
}
std::swap(unresolved, dependencies);
return unsolved();
}
const std::string getName() const { return name; }
private:
std::string name{};
mutable unsigned value = 0;
mutable std::vector<std::string> dependencies{};
};
bool operator<(const Variable &v, const std::string &s) {
return v.getName() < s;
}
bool operator<(const std::string &s, const Variable &v) {
return v.getName() < s;
}
std::set<Variable, std::less<>> solve(std::istream &in) {
std::set<Variable, std::less<>> equations;
bool unsolved = false;
for (Variable var; in >> var; ) {
unsolved |= var.unsolved();
equations.insert(var);
}
while (unsolved) {
unsolved = false;
for (auto &eq: equations) {
unsolved |= eq.replaceKnowns(equations);
}
}
return equations;
}
int main() {
std::stringstream in{"b = c + d + 3\nd = e + 4\na = b + c + d + 1\ne = 7\nc = d + 2"};
auto solution_list = solve(in);
for (const auto& s: solution_list) {
std::cout << s << "\n";
}
}
Sample input and output
The main
function tests the following system of equations:
b = c + d + 3
d = e + 4
a = b + c + d + 1
e = 7
c = d + 2
This results in the following correct output:
a = 52
b = 27
c = 13
d = 11
e = 7
std::less<>
, for starters). \$\endgroup\$std::less<void>
approach that is currently in the code? \$\endgroup\$std::less<>
\$\endgroup\$