# Parsing numbers from equations into strings

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 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 << equation1 << endl;

init();
}

void init() {
size_t firstx = equation1.find_first_of("x");
size_t secondx = equation1.find_first_of("x", firstx + 1);
if (secondx == string::npos) {
cout << "Secondx == 0" << endl;
}

//cout << firstx << endl;
//cout << secondx << endl;

int startloc = firstx - 1;

while (true) {
//cout << "starting with startloc = " << startloc << endl;
if (startloc == -1) {
break;
}
char c = equation1.at(startloc);
if (c == ' ' || c == '=' || c == '+' || c == '-' || c == '*' || c == '/') {
//cout << "Found something" << endl;
break;
}
startloc--;
}

string s;
unsigned int i = startloc + 1;
//cout << i << endl;
//cout << firstx << endl;
while (i < firstx) {
//cout << "Character at " << i << endl;
//cout << equation1[i] << endl;
stringstream ss;
string temp;
ss << equation1[i];
ss >> temp;
s.append(temp);
i++;
}
cout << s << endl;

startloc = secondx - 1;

while (true) {
//cout << "starting with startloc = " << startloc << endl;
if (startloc == 0) {
cout << "Problem" << endl;
}
char c = equation1.at(startloc);
if (c == ' ' || c == '=' || c == '+' || c == '-' || c == '*' || c == '/') {
//cout << "Found something" << endl;
break;
}
startloc--;
}

s = "";
i = startloc + 1;
while (i < secondx) {
stringstream ss;
string temp;
ss << equation1[i];
ss >> s;
s.append(temp);
i++;
}

cout << s << endl;
}

};

int main(int argc, char *argv[])
{
Equation equation(argv[1], "blank");
//printHelp();
string s;
cin >> s;
return 0;
}

void printHelp()
{
cout << "Welcome!" << endl;
cout << "Stuff about commands" << endl;
}

-
The main issue in this program is the program design. Most importantly, you need to use the most fundamental concept in object-oriented design: private encapsulation. C++ is no different from Java in this. Since you are a beginner programmer, you should learn object-oriented design as early as possible, this is far more important than learning all the dirty details of a particular language. I'd actually go back to Java and study OO design with that language as foundation, since Java is far cleaner (though less powerful) than C++. – Lundin Jan 30 '13 at 15:46
Comment your code. What's the purpose of the code? What's the main algorithm? What are the important lines of code and why do they exist? – Dave Jarvis Jan 30 '13 at 17:19
Thanks guys. I dont think i need to go back and relearn OOP because im sure i understand it, althought this code doesnt show it. I wrote this in about 15 minutes so i didnt clean it at all. Im going to repost it once i clean it in 2ish hours. (Also, i dont understand pointers, if anyone has advice) hopefully it will look alot nicer when cleaned – Tips48 Jan 30 '13 at 18:23
Updated the code, attempted to clean it up and use some of the changes that were suggested – Tips48 Jan 30 '13 at 21:24
My code has been cleaned up greatly, thank you guys. – Tips48 Jan 31 '13 at 1:01

Without changing things too much...

• 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.

-
Thanks, lots of little things i didnt think of. As for printhelp() ill be using it when i start expanding the program. Matrix[][] is unused but i will be using it later. The reason i have the equation class is because ill be expanding it to do multiple things later. I just wrote some (messy) code to try to get the splitting. Your saying instead of #include <string> use std::string? O.o – Tips48 Jan 30 '13 at 11:52
I understand putting that loop in a function since its used multiple times. My last question: is that char -> string conversion correct? I couldnt find any way to do it faster – Tips48 Jan 30 '13 at 11:57
You need #include <string> anyway. But use std::string rather than using std and string. – Glenn Rogers Jan 30 '13 at 12:14
re the conversion, I don't want to even look at it!! For your loops, the first loop is doing find_last_of, and the second is doing substr. Use these instead of making your own. – Glenn Rogers Jan 30 '13 at 12:20
I get how you can use substring and wish i remembered earlier, oops! But how would you use find_last_of? I dont really get how i would use it to replace the first loop. – Tips48 Jan 30 '13 at 15:04