0

Goal: Android/iOS app that displays tricky-to-spell words when user puts in a letter.

The code runs correctly the first time, but after asking the user if they want to enter another letter, the list of words from the file are displayed multiple times. The number of times the list prints is incrementing ex. 1,2,3...

Expected result: List of words for one letter will print once

Actual result: List of words prints more than once every time the program loops. Increments with i variable.

Error messages: None

Tried:

  1. Modifying printWords() by using a decrement.

    if (i == 2){ i--; }

  2. https://www.youtube.com/watch?v=Iho2EdJgusQ

  3. Why does the answer print out twice?
  4. http://www.cplusplus.com/forum/beginner/199381/
  5. https://codereview.stackexchange.com/questions/139841/printing-out-vectorstring-char-by-char
  6. Changed do-while to while

Code: 

/* 
Description: Android/iOS application that takes in a letter and displays 
tricky-to-spell words.
*/

#include "includeDeclarations.h"
#include "usingDeclarations.h"

char userLetterInput;
char userChoiceContinue;
string userFirstName;
string line;
vector <string> trickyWordsVector;
void printWords();

int main() {
    cout << "----------------<>-----------\n";
    cout << "Welcome to Your TRICKY WORDS Helper!\n";
    cout << "----------------<>-----------\n";
    cout << "\n\nEnter your first name: ";
    cin >> userFirstName;

  do {
    cout << "\nEnter a letter [a - z]: ";
    cin >> userLetterInput;
    userLetterInput = toupper(userLetterInput);

    if(isalpha(userLetterInput)){
        cout << "\n" << userFirstName << ",\n\nHere's your list of tricky words for the letter (" << char(toupper(userLetterInput)) << "):\n" << endl;

    ifstream trickyWordsFile("trickyWordsList.txt");

    if (trickyWordsFile.is_open()) {
        while (getline(trickyWordsFile, line)) {
            if (line.size() > 0) {
                trickyWordsVector.push_back(line);
          }
      }
    } else {
        cerr << "Cannot open the file.";
    }

    trickyWordsFile.close();

    printWords();
    }

    cout << "\nWould you like to enter another letter [y,n]?: "; //TODO data validation
    cin >> userChoiceContinue;
} while(char(tolower(userChoiceContinue)) == 'y');

  cout << "\n----------------<>-----------\n";
  cout << "Thank you for using Your TRICKY WORDS Helper!\n";
  cout << "----------------<>-----------\n";

return 0;
} // end main()

void printWords(){

     //TODO error prints list more than once.
     for (int i = 0; i < trickyWordsVector.size(); i++) {

     if(trickyWordsVector[i][0] == userLetterInput){
     cout << trickyWordsVector[i];
     cout << "\n";
     }
    }
} // end printWords()

usingDeclarations.h

#define cout std::cout
#define cin std::cin
#define getline std::getline
#define endl std::endl
#define string std::string
#define ifstream std::ifstream
#define cerr std::cerr
#define vector std::vector

includeDeclarations.h

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

trickyWordsList.txt or trickyWordsFile

Argument 
Atheist 
Axle 
Bellwether 
Broccoli 
Bureau 
Caribbean 
Calendar
Camaraderie 
Desiccate 
Desperate 
Deterrence

Thanks for any advice.

Run code using https://repl.it/~

Electric Egghead
  • 19
  • 4
  • 2
    Have you tried to step through your code, statement by statement, in a debugger? – Some programmer dude Mar 11 '20 at 11:07
  • 1
    your `usingDeclarations.h` is ...not nice. First you use macros which alone is not good but then the name of the header suggests that it contains `using` declarations which it does not. Use `using std::cout;` etc instead of the macros – 463035818_is_not_an_ai Mar 11 '20 at 11:07
  • 1
    As a hint about your problem: How many times are you reading into the `trickyWordsVector` vector? And how many times do you need to do it? – Some programmer dude Mar 11 '20 at 11:09
  • 1
    still not the problem, but consider you write a function `std::string getline ( foo& f);`, you cannot because the macros mess up your syntax – 463035818_is_not_an_ai Mar 11 '20 at 11:10
  • 2
    You forgot The Golden Rule Of Computer Programming: "your computer always does exactly what you tell it to do instead of what you want it to do". You told your computer: "every time I enter the letter, read the file and add its contents to the vector". Your computer is merely following your instructions: with every letter, it adds the contents of the file to the vector. After the 2nd letter, that'll be done twice, after the 3rd letter, three copies of the file are in a vector. If you want your computer to do something else, you'll have to tell your computer what to do, exactly. – Sam Varshavchik Mar 11 '20 at 11:10
  • 1
    Suggestion: Rewrite the whole program without using one single global variable. Use a suitable (narrow) scope for each local variable. Call functions by passing the variables (by value or by reference) it needs as arguments. – Ted Lyngmo Mar 11 '20 at 11:39
  • @Someprogrammerdude: Thanks for the advice. I can step through the code using Microsoft VS. Reading into the vector? I'll think about the solution. – Electric Egghead Mar 11 '20 at 15:22
  • @idclev463035818 Thanks! I don't know about headers or macros. I wanted to avoid adding the using namespace std; to my code. I read it was bad practice. I'll look up these headers and try those. I don't know anything about 'foo' yet. I need to do more research, lol. – Electric Egghead Mar 11 '20 at 15:25
  • @SamVarshavchik I know! My poor program has the wrong instructions. It is SO lost. Hmm... all right. I went through the code, trying to figure out the logic, but fixing it was beyond me. – Electric Egghead Mar 11 '20 at 15:27
  • @TedLyngmo Could you explain more about the global variables? Are they too far away from the section where I used them? How many variables can I pass through a function and the code is still readable? Thanks, Ted! – Electric Egghead Mar 11 '20 at 15:29
  • Thanks for your comments. Is there anything else terribly wrong with this code? All suggestions appreciated. – Electric Egghead Mar 11 '20 at 15:31
  • no problem if you dont know about macros, the only thing you need to know is: You dont need them ;). Only for header guards (search for it if you dont know what it means). Not using `using namespace std;` is good, but as I wrote, then just use what you need `using std::cout;` (instead of the full `std` namespace that you would get with `using namespace std;`) – 463035818_is_not_an_ai Mar 11 '20 at 15:32
  • @ElectricEgghead Far away doesn't matter much. If you make a class, the declaration of member variables is often far away from where they are used. The problem is that it's hard to get a good overview of how and where they are used if they are global. You can typically pass "a few" variables to a function and still have it readable. If you need to pass 10 or more I'd say that you probably should refactor your code somehow. A function's purpose should typically be to do one thing only - and to do it good. To achieve that, a dozen parameters is often counter productive – Ted Lyngmo Mar 11 '20 at 15:48

2 Answers2

1

Just add 

   trickyWordsVector.clear();

after 

   do

statement it removes all elements from the vector, Because The old copies are always there.

srilakshmikanthanp
  • 2,231
  • 1
  • 8
  • 25
0

I used Sri's answer above and it worked, but I wanted to figure out my own solution.

After a few hours, I solved the problem by doing this:

//added the second line to the code
if (trickyWordsFile.is_open()) {
  if (trickyWordsVector.empty()) {
    while (getline(trickyWordsFile, line)) {
        if (line.size() > 0) {
            trickyWordsVector.push_back(line);
            }
         }
     }
}

Onto applying the other suggestions and rewriting the whole program without using global variables. :)

Any other suggestions are appreciated.

Electric Egghead
  • 19
  • 4