views:

372

answers:

6

Hes my code and I can't figure out where I'm getting the divide by zero problem.

mreviewApp.cpp

const int SIZE = 80;
const char DELIMIT = '|';

void parseLine(const char line[], string& title, int& rating);
void stringTrim(char st[]);
void printMrList(std::vector <Mreview> mrList);
Mreview searchTitle(std::vector <Mreview> &mrList, string title);


int main()
 {
  ifstream fin;

  fin.open("rating_list.txt");
  if (fin.fail()) {
    cerr << "Input file opening error.\n";
    exit(1);
  }

  char line[SIZE];
  string title;
  int rating;
  int lineCount = 0;

  std::vector <Mreview> mrList;

  /* Process one line at a time */
  // Read the first line
  fin.getline(line, SIZE);
  stringTrim(line);

  // Process loop
  while (strlen(line) != 0) { 
    parseLine(line, title, rating);
    lineCount++;

    Mreview review = searchTitle(mrList, title);
    review.addScore(rating);

    // Read the next line
    fin.getline(line, SIZE);
    stringTrim(line);
  }

  cout << "** PROCESS DONE. There were " << mrList.size() << " movie titles. **\n\n";

  printMrList(mrList);
  // Close the input file before exit.
  fin.close();

  system("Pause");
  return 0;
}

void parseLine(const char line[], string& title, int& rating)
{
  char s[SIZE], r[SIZE];
  const char *ptr, *temp1;
  char *temp2;

  ptr = strchr(line, DELIMIT);

  if (ptr != NULL) {
    // First grab the title string (until '|').
    temp1 = line;
    temp2 = s;
    while (temp1 != ptr)
      *temp2++ = *temp1++;

    *temp2 = '\0';

    stringTrim(s);
    title = s;

    // Second grab the rating number
    temp1 = ptr+1;
    temp2 = r;
    while (*temp1 != '\0')
      *temp2++ = *temp1++;

    *temp2 = '\0';

    stringTrim(r);
    rating = atoi(r);
  }
  else {
    title = "";
    rating = 0;
  }
}

void stringTrim(char st[])
{
  char* ptr;

  for (ptr = st; *ptr; ptr++) ;
  for (ptr--; *ptr == ' ' && ptr >= st; ptr--) ;
  ptr++;
  *ptr = '\0';

  for (ptr = st; *ptr && *ptr == ' '; ptr++) ;

  if (*ptr && ptr != st) {
    char* ptr2;
    for (ptr2 = st; *ptr; *ptr2++ = *ptr++) ;
    *ptr2 = '\0';
  }
}

void printMrList(std::vector <Mreview> mrList)
{
    std::vector<Mreview>::iterator itr;
    for(itr = mrList.begin(); itr != mrList.end(); itr++) {
     Mreview review = *(itr);
     cout << review.getTitle() << "\t\t" << review.getTotalScore() << "\t\t" << review.aveRating() << endl;
    }
}

Mreview searchTitle(std::vector <Mreview> &mrList, string title)
{
    Mreview review (title); 
    std::vector<Mreview>::iterator itr;
    for(itr = mrList.begin(); itr != mrList.end(); itr++) {
     Mreview r2d2 = *(itr);
     if(review.getTitle() == r2d2.getTitle())
      return r2d2;
    }
    mrList.push_back(review);
    return review;
}

mreview.cpp

Mreview::Mreview(string ttl)
  : title(ttl), totalScore(0), numRatings(0) {}

Mreview::Mreview(string ttl, int score)
  : title(ttl), totalScore(score), numRatings(1) {}
void Mreview::addScore(int score)
{
    this->totalScore += score;
    this->numRatings += 1;
}

double Mreview::aveRating() const
{
    double rating = totalScore/numRatings;
    return rating;
}

mreview.h

#ifndef MREVIEW_H
#define MREVIEW_H

#include <string>
using namespace std;

class Mreview
{
public:
  Mreview(string ttl = "N/A");
  Mreview(string ttl, int firstScore);

  string getTitle() const { return title; }
  int getTotalScore() const { return totalScore; }
  int getNumRatings() const { return numRatings; }

  void addScore(int score);
  double aveRating() const;

private:
  string title;
  int totalScore;
  int numRatings;
};

#endif

My problem is that I am unable to figure out what I have to do to fix the problem. I have read the comments and I am still confused.

+3  A: 

I only see one actual division operation...

double Mreview::aveRating() const { 
   double rating = totalScore/numRatings; 
   return rating; 
}

...so is it there?

matt b
+3  A: 

When looking for divide by zero problems, look for / and % operators. I see only one / in your code.

Paul Beckingham
+2  A: 

Can you run it in a debugger and let it tell you where the trap occurred?

Matt J
+3  A: 

Not the divide by zero error, but...

double Mreview::aveRating() const
{
    double rating = totalScore/numRatings;
    return rating;
}

You are taking two integer values and storing it in a double without typecasting... you have to cast either totalScore or numRatings to a double like:

double Mreview::aveRating() const
{
    double rating = (double)totalScore/numRatings;
    return rating;
}
DeadHead
+6  A: 

You're allocating (and copying!) a vector of Mreview objects, all of which are being constructed with the default ctor, so numRatings defaults to 0, and you are doing nothing to ensure that aveRating() is never called on unmodified objects or at the very least protects itself from numRatings being 0.

Edit: Easiest fix:

double Mreview::aveRating() const { 
    if (numRatings == 0)
        return 0.;
    else 
        return static_cast<double>(totalScore)/numRatings;
}

A further fix, IMO, would be to store Mreview pointers (ideally shared_ptr) in the vector and newing them when the first score is to be added rather than earlier.

greyfade
I'm just confused on how exactly to fix this issue.
Jim
Do Paul's suggestion. That's the easiest fix. Personally, I wouldn't store this kind of class in a vector without pointers anyway: I'd make the default ctor explicit and only add a new instance of the class to the vector<> when I have a firstScore to put in it.
greyfade
Er, rather, Paul's suggestion testing numRatings instead of totalScore. Sorry.
greyfade
+1  A: 

It looks like if the length of the line is 0 after trimming, then printMrList() can be called without review.addScore() being called first. In that case, printMrList() calls review.aveRating() which tries to divide totalScore by numRatings. Both of these values are 0, so a divide by zero error occurs.

jeffm