-3

Currently have this simple code written, trying to figure out why it's not counting specific words every time.

#include "pch.h"
#include <fstream>
#include <string>
#include <iostream>

using namespace std;
int main()
{
    int t = 0;
    int a1 = 0;
    string a[100];
    ifstream food;

    food.open("food.txt");

    if (food.fail())
    {
        cout << "File can't open" << endl;

    }

    else
        cout << "File successfully opened" << endl;

    int i = 0;

    while (!food.eof())
    {
        // Tomato
        food >> a[i];
        if (a[i] == "tomato")
        {
            t = t + 1;
        }
        i++;

        // Apple
        food >> a[i];
        if (a[i] == "apple")
        {
            a1 = a1 + 1;
        }
        i++;
    }
    cout << "Amount of Tomatos: " << t << endl;
    cout << "Amount of Apples: " << a1 << endl;
}

The text file I'm using:

apple
apple
tomato
apple
tomato
tomato

The output:

File successfully opened
Amount of Tomatoes: 2
Amount of Apples: 2

The purpose is to find the amount of each food found in the list. I'm currently only using two kinds of food but will have many more.

Remy Lebeau
  • 454,445
  • 28
  • 366
  • 620
Cas_
  • 3
  • 3
  • [Why is iostream::eof inside a loop condition considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong) – πάντα ῥεῖ Nov 13 '18 at 22:20
  • 2
    i suggest to read it to your rubber duck. It should raise an eyebrow or two once you reach the "I read one food, if its an apple I increment the apple counter, then I read the next food, if its a tomato, I increment the tomato counter, then I repeat" part... – 463035818_is_not_a_number Nov 13 '18 at 22:24
  • After re-looking at it and looking at other comments, I realized what I was doing wrong with the if and else statements. Does sound odd now that I realize my mistake. – Cas_ Nov 13 '18 at 22:38
  • Since the quantity of words is unknown at run-time, use `std::vector` instead of an array. Arrays can overflow. Large arrays may be a waste of memory. – Thomas Matthews Nov 13 '18 at 23:47

1 Answers1

2

There are several problems with your code.

  • using eof() in a loop incorrectly. You can't check eof() before performing a read operation first.

  • using an array without bounds checking. For that matter, you don't need an array at all.

  • skipping words, which is why you are not counting everything you are expecting. Let's take the very first line, apple. Since it is not a "tomato", you skip it and read the next word in the file, which is "apple", so you count it. But you didn't count the 1st apple at all.

You need to do something more like this instead:

#include "pch.h"
#include <fstream>
#include <string>
#include <iostream>

using namespace std;

int main()
{
  int tomatoes = 0;
  int apples = 0;
  string s;
  ifstream food;

  food.open("food.txt");

  if (!food.is_open())
  {
    cout << "File can't open" << endl;
    return 0;
  }

  cout << "File successfully opened" << endl;

  while (food >> s)
  {
    // Tomato
    if (s == "tomato")
        ++tomatoes;

    // Apple
    else if (s == "apple")
        ++apples;
  }

  cout << "Amount of Tomatos: " << tomatoes << endl;
  cout << "Amount of Apples: " << apples << endl;

  return 0;
}

Alternatively, as @user463035818 mentioned in comments, you can use a std::map instead:

#include "pch.h"
#include <fstream>
#include <string>
#include <iostream>
#include <map>

using namespace std;

int main()
{
  map<string, int> foods;
  string s;
  ifstream food;

  food.open("food.txt");

  if (!food.is_open())
  {
    cout << "File can't open" << endl;
    return 0;
  }

  cout << "File successfully opened" << endl;

  while (food >> s) {
    foods[s]++;
  }

  for (map<string, int>::iterator iter = foods.begin(); iter != foods.end(); ++iter) {
      cout << "Amount of " << iter->first << ": " << iter->second << endl;
  }

  /* or, if you are using C++11 or later...
  for (auto &item : foods) {
      cout << "Amount of " << item.first << ": " << item.second << endl;
  }
  */

  return 0;
}
Remy Lebeau
  • 454,445
  • 28
  • 366
  • 620
  • I really see where I over did a few things and did notice the if and else if was very important. Thanks for the help! – Cas_ Nov 13 '18 at 22:32
  • Addendum: Also note how Remy used variable names that described what they counted. This is very helpful in reducing bugs, as it's easier to to see where you accidentally used `apples` in place of `tomatoes` than `t` and `a1`. In addition you spend less time looking up the meaning of `t` and `a1` when debugging. – user4581301 Nov 13 '18 at 22:32
  • @user4581301 I suppose yes, especially since I'm still fairly new to C++. Wasn't sure if using this website would help me understand things easier, but I'm confident in that now. – Cas_ Nov 13 '18 at 22:36