How do you know when you have it "right"?

[Jahmbo]

I am totally new to C++, I studied assembler and C in college about 15 years ago and have worked with asp, dhtml, jscript, vbscript etc... over the years. So far all I have managed to do is gather together some 'training materials' and start at the beginning. One of the books I am starting with is Bruce Eckels 'Thinking in C++ Volume 1 (2nd Ed.)

So here's my question, at the end of chapter 2 he assigns a few exercizes

-Create a program that opens a file and counts the whitespace-separated words in that file.
-Create a program that counts the occurrence of a particular word in a file (use the string class’ operator ‘==’ to find the word).

Rather than do what he asked (my normal state is to do what I want) I decided to prompt the user for a file and a word to search for, then search the file replacing words with periods unless they match, count the matches, display which number word it is in the file that is matched, show total words, yada yada.

Would anyone care to take a look at the code and tell me if it could be cleaned up and what directions I might go in an effort to clean it up?

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

using namespace std;

int main() {
    
    vector<string> words;
    string word, searchterm;
    char filename[50];
    int matchnum = 0;
    int finalcount;
    
    ifstream infile;
    cout << "A Funky Word Searcher/Counter:" << endl << endl;
    cout << "Enter the filename of the text file you wish to process:> " ; 
    cin.getline(filename, 50);
    infile.open (filename);
    cout << "Enter the word you wish to search/count:> " ;
    cin >> searchterm; 
    if(!infile.is_open())
    exit(EXIT_FAILURE);

    cout << "You are searching for the word '" << searchterm << "' in the file '" 
    << filename << "'" << "\n" << "\n" << endl;
    system("pause");

    while(infile >> word)
    words.push_back(word);

    for(int i = 0; i < words.size(); i++){
        if (words[i] == searchterm)
           cout << "\n" << "Word # " << i << ": '" << words[i] << "' match! # " << ++matchnum << endl;
        else
    cout << "." ;
    finalcount = i;
    }
    
    cout << "\n" << "\n" << "\n" << matchnum << " total occurence(s) of " << "'" 
    << searchterm << "'" << " in the file " << "'" << filename << "' out of " << finalcount << " total words." << endl;
    cout << "\n";

        system("pause");
        return 0;
    
} ///:~

Cheers and Salutations to all, glad to have found this site and its resources.

Jahmbo

[chrisname]

I have two main suggestions here:
1. Some of your indentation is messy (in two places you don't indent a block (lines 23 and 30) and in one you actually did indent the block, but then un-indented it before the block was finished (line 33))
2. You used system("pause"): http://cplusplus.com/forum/articles/11153/

I also have another couple of suggestions which aren't very important; but you should still take them into account:
1. For very large functions (more than a screenful or two of text (I'm talking about an ANSI terminal screenful (24 lines)) you should try to make several "helper" functions. This is also useful because it splits your code into logical blocks, making it easier to understand. If your program can't afford to lose speed with function call overhead, maybe you could inline the other functions (only if they're a few lines long, though).
2. There's no need to use EXIT_SUCCESS and EXIT_FAILURE; you can just use 0 and n respectively (where n is a number that isn't 0).

Other than that; there's nothing wrong with what I saw; although admittedly I only read your code quickly.

[billinares]

Hi

I actually did that exercise not long ago! It is not as simple a one might think (the whitespace separation). You got to take into acount things such as when two ore more whitespaces are next to eachother, lines with only whitespace, etc. Actually the best way to do it would probably be using a state machine. But I did it with just if-statements.

[mmthob]

if you are using codeblock, dont use "system ("pause")" at the end just before return 0,because it is unnecessary.