patterncppModerate
Parsing numbers from equations into strings
Viewed 0 times
equationsintonumbersparsingfromstrings
Problem
This is my first code in C++. Since I'm new to the language, I'm just looking for pointers on what can be made better. I tried to cut out unnecessary stuff, but there are some comments in there. I know Java, so if you're trying to explain anything in Java, I can understand it.
Feel free to mention anything, whether it be code style (which I'm completely unsure what the conventions are in C++), performance, etc.
The goal of the program is to take an equation, such as
Feel free to mention anything, whether it be code style (which I'm completely unsure what the conventions are in C++), performance, etc.
The goal of the program is to take an equation, such as
5x^2+48204x=7, and parse the 5 and 48204 into strings. using namespace std;
void printHelp();
class Equation {
public:
string equation1;
string equation2;
int matrix [3][2];
Equation(string one, string two) {
one.erase(remove_if(one.begin(), one.end(), isspace), one.end());
two.erase(remove_if(two.begin(), two.end(), isspace), two.end());
equation1 = one;
equation2 = two;
cout > temp;
s.append(temp);
i++;
}
cout > s;
s.append(temp);
i++;
}
cout > s;
return 0;
}
void printHelp()
{
cout << "Welcome!" << endl;
cout << "Stuff about commands" << endl;
}Solution
Without changing things too much...
-
You need some
-
Don't use
-
You might as well put
-
Matrix also isn't used.
-
Having an
-
You don't allow for the possibility that
-
If
-
Multiple breaks in while loops should be avoided, for readability. This (first) loop probably should be split off into another function since it's used twice, together with the following section.
-
I'm a little nervous of
-
Consider using std::string::find_last_of and std::string::substr. They'll simplify the code somewhat.
-
Check that you don't lose a negative coefficient.
-
You need some
#includes.-
Don't use
using namespace. One of the issues is knowing where a function comes from (which one is being used) - this hides it. Using a short std:: gains you a lot in understandability. You can do using std::string which is better, but imo even that isn't worth it.-
You might as well put
printHelp before main rather than after, that way you don't have to declare it. And it's not used currently anyway. -
Matrix also isn't used.
-
Having an
Equation class doesn't make sense for what you're (currently) using it for. You might as well just have a naked function.-
You don't allow for the possibility that
firstx may be invalid, ie there are no x in the equation. This may be a given, of course. And for that matter, that the x^2 term will be first!-
If
secondx is invalid, skip over processing it.-
Multiple breaks in while loops should be avoided, for readability. This (first) loop probably should be split off into another function since it's used twice, together with the following section.
-
I'm a little nervous of
int startloc = firstx - 1;, since firstx is unsigned and startloc isn't. I'd cast firstx to an int first if I were using it like this.-
Consider using std::string::find_last_of and std::string::substr. They'll simplify the code somewhat.
-
Check that you don't lose a negative coefficient.
Context
StackExchange Code Review Q#21048, answer score: 10
Revisions (0)
No revisions yet.