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.
-
I only see one actual division operation...
double Mreview::aveRating() const { double rating = totalScore/numRatings; return rating; }
...so is it there?
-
When looking for divide by zero problems, look for / and % operators. I see only one / in your code.
-
Can you run it in a debugger and let it tell you where the trap occurred?
-
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; }
-
You're allocating (and copying!) a vector of
Mreview
objects, all of which are being constructed with the default ctor, sonumRatings
defaults to 0, and you are doing nothing to ensure thataveRating()
is never called on unmodified objects or at the very least protects itself fromnumRatings
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 andnew
ing them when the first score is to be added rather than earlier.Jim : I'm just confused on how exactly to fix this issue.greyfade : 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. -
It looks like if the length of the line is 0 after trimming, then
printMrList()
can be called withoutreview.addScore()
being called first. In that case,printMrList()
callsreview.aveRating()
which tries to dividetotalScore
bynumRatings
. Both of these values are 0, so a divide by zero error occurs.
0 comments:
Post a Comment