Critique my code

[RastaWolf]

Hey everyone,

First off, I'm a superN00b programmer so this is not very advanced.

I just finished a project that does some textual analysis by exploding strings into a vector and I'd like to have some feedback. The point of this program was to practice with string member functions and to become a better coder ( I am not a student ).

All critiques are welcome as they only help me become a better coder. I will take everyone's comments seriously and try my best to keep improving.

Thanks for the feedback community!

#include <iostream>
#include <string>
#include <vector>

using namespace std;

bool isMax(size_t w1, size_t w2) {
	if(w1 > w2)
		return true;
	else
		return false;
}

bool isEqual(size_t w1, size_t w2){
	if(w1 == w2){
		return true;
	}
	else return false;
}


int main(){
	string s;
	vector<string> word;
	cout << "Enter a string: ";
	getline(cin, s);
	size_t found;
	size_t found1;
	found = s.find_first_not_of(" ");
	found1 = s.find_first_of(" ");
	while(found1 != string::npos){
		word.push_back(s.substr(found,found1-found));
		found = found1 + 1;
		found1 = s.find_first_of(" ", found1 + 1);
	}
	found1 = string::npos;
	int count = 0;
	word.push_back(s.substr(found, found1));
	unsigned int i = 0;
	for (i = 0; i < word.size(); ++i ){
		if (word[i].substr(word[i].length() - 1) == ".")
			word[i] = word[i].substr(0, word[i].length() - 1);
		if (word[i].substr(word[i].length() - 1) == ",")
			word[i] = word[i].substr(0, word[i].length() - 1);
	}
	string sWord;
	cout << "What word would you like to search for?: ";
	cin >> sWord;
	unsigned int max = 0;
	for (unsigned int i = 0; i < word.size(); ++i){
		if(word[i] == sWord)
			++count;
	}
	i = 0;
	vector<size_t> equals;
	if (isEqual(word[i].length(), word[i+1].length())){
		equals.push_back(i);
		equals.push_back(i+1);
	}

	if (isMax(word[i].length(), word[i+1].length()))
		max = i;
	if (!isMax(word[i].length(), word[i+1].length()))
			max = i+1;
	i = 2;
	do {
		if (max == i) i += 1;
		if (isEqual(word[max].length(), word[i].length())){
				equals.push_back(max);
				equals.push_back(i);
		}
		if ( !isMax(word[max].length(), word[i].length() ))
		{
		    max = i;
		}
	i++;
	} while (i < word.size() );
	i = 0;
	vector<string> maxW;
	maxW.push_back(word[max]);
	if (equals.size() != 1) {
	for(i = 0; i < equals.size(); ++i){
		if(word[max].length() == word[equals[i]].length()){
			if (word[max] == word[equals[i]])
				++i;
			else
			maxW.push_back(word[equals[i]]);
		}
	}
	}
	i = 0;
	if ( maxW.size() != 1){
		cout << "\nThe strings with the most characters are ";
		while (i < maxW.size() ){
			cout << maxW[i];
			if (i != maxW.size() - 1)
				cout << " & ";
			else{
				cout << ". ";
				break;
			}
			i++;
		}
	}
	else {
		cout << "\nThe string with the most characters is " << maxW[maxW.size() - 1] << ".";
	}
	if (count == 1)
		cout << "\nThere is " << count << " instance of the word " << sWord << "." << endl;
	else
		cout << "\nThere are " << count << " instances of the word " << sWord << "." << endl;

	return 0;
}

[Athar]

Constructs like

if(w1 > w2)
		return true;
	else
		return false;

don't serve any purpose, because operator> already returns a bool.
It's the same as:
return w1 > w2;

What is the purpose of the functions isMax and isEqual anyway? Why aren't you using > and == directly?

[RastaWolf]

There really was no purpose of the function. I just felt like since I didn't have one, I should write one. Guess that's not a very good reason. I remember from one of the chapters I was reading in a c++ book that I could write bool functions, so I was "putting it to the test."

I guess bool returning functions are useful when you need a bool value that logical operators can't yield by themselves, or when you're defining classes.


Thank you very much for reading my code Athar!

[Athar]

Also, you should not reuse counter variables like i. This is confusing and makes it less apparent what type i is.
Here you are using a while loop where you should use a for loop:

i=0;
[...]
while (i < maxW.size() ){
    [...]
    i++;
}

[RastaWolf]

So every time I want to use 'i' as a counter I should use a for loop?

I can see what you mean by it being confusing what type 'i' is with it being reassigned.

I'll work on making those changes.

Thanks for the tip!