Jump to content
  • Advertisement
Sign in to follow this  
YAITS

Help me kill this bug in my assessment!!

This topic is 4754 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi, im a uni student(Yet another IT student) we were asked to write a simple program in c++, that reads "student id and marks", then sort them to different bands like "credit, pass" etc. everything is fine, but one part, the program just skips all the codes, but the code looks fine to me. so here i simplified my code.
#include<iostream>
#include<math.h>
using namespace std;

const int rec=300;
int istuid[rec];//student id
float fmark[rec];//student mark

void vinput(int & i);
void vdisplay(int & i);

int main(void)
{
    int i=0;
    vinput(i);
    vdisplay(i);
    system("pause");
}

void vinput(int & i)
{
     char yn; //variable for yes or no
     while (!cin.eof() && i < rec)
     {
           cin >> istuid;
           if (cin.eof()){i++;continue;} 
           else if (!(istuid < 999999 && istuid > 0))
           {
                cout << "Error, Student ID is invalid\n"
                     << "Do you still want to display the result(Y/N)";
/***********************************************
ERROR here
after running this line, it skips the "cin" >_<
***********************************************/
                cin >> yn;
                if (yn != 'y' || yn != 'Y')
                {
                    cout << "\nprogram terminates, ";
                    system("pause");
                    exit(1);
                }
                break;
           }
           cin >> fmark;
           if (cin.eof()){i++;continue;}
           else if (!(fmark < 100 && fmark > 0))
           {
                cout << "Error, Student mark is invalid\n"
                     << "Do you still want to display the result(Y/N)";
                cin >> yn;
                if (yn != 'y' || yn != 'Y')
                {
                    cout << "\nprogram terminates, ";
                    system("pause");
                    exit(1);
                }
                break;
           }
     i++; //i plus plus here, its not showing i dono why
     }
}

void vdisplay(int & i)
{
     int pert=0;
     double average=0,subtra,sd=0,sump=0;
     
     //finding how many records and average
     for (int j = 0; j < i; j++)
     {
         if (fmark[j])
         {
         pert++;
         average += fmark[j];
         }
     }
     if (pert > 0)
     {
          average = average / pert;
          cout << "Pertinent records:\t" << pert <<"\n";
          cout << "Mean or average:\t" << average <<"\n";
     //end number of records & average
     
     //start loop again for standard deviation
          for (int j = 0; j < i; j++)
          {
              if (fmark[j])
              {
              subtra = fmark[j] - average;
              sump += pow(subtra, 2);
              }
          }
          sd = sump / pert;
          sd = sqrt(sd);
          cout << "Standard deviation:\t" << sd <<"\n\n";
     //end finding standard deviation
     
     //loop last time for displaying records
          for (int j = 0; j < i; j++)
          {
              if (fmark[j])
              {
              cout << istuid[j] << "\t" <<fmark[j] << "\n";
              }
          }
     }
}

please test this and enter something in like 2364 76 S

Share this post


Link to post
Share on other sites
Advertisement
Just from looking at that, I think I see the problem - let me ask you what happens when you get input for an integer, then try and get input for a character? [wink] You must get rid of that extra endline that is entered in when you press return after entering an integer. I'm sure you can figure out what to do now...

Share this post


Link to post
Share on other sites
extra endl?
but if i keep entering integers, it works fine after ending by ^z

Share this post


Link to post
Share on other sites
its because this

if(yn != 'y' || yn != 'Y')
{
cout << "\nprogram terminates, ";
system("pause");
exit(1);
}



should be

if(yn != 'y' && yn != 'Y')
{
cout << "\nprogram terminates, ";
system("pause");
exit(1);
}





also, please use this for your pauses (its os independent)

void pause()
{
// wait for enter
char a;
std::cin >> a;
}


Share this post


Link to post
Share on other sites
Oh logic mix-up. If that's true I would actually argue that it should be this.

if(!(yn == 'y' || yn == 'Y'))

Share this post


Link to post
Share on other sites
theres so much more I would rip apart than
!(yn == 'y' || yn == 'Y') = (yn != 'y' && yn != 'Y)

Share this post


Link to post
Share on other sites
i tied cin.clear() but it wont work.
and even if "if condition" have problem, the real problem is that it skips cin >> yn;

but thx for everyone help

Share this post


Link to post
Share on other sites

#include<iostream>
#include<math.h>
using namespace std;

const int rec=300;
int istuid[rec];//student id
float fmark[rec];//student mark

// Why are you limiting things this way, and adding the complexity of having to
// count the records yourself?

void vinput(int & i);
void vdisplay(int & i);

// what the hell does the v stand for? Also, why are you using the parameter for
// a function return value, instead of, well, the return value?

int main(void)
{
int i=0;
vinput(i);
vdisplay(i);
system("pause");
}

void vinput(int & i)
{
char yn; //variable for yes or no
while (!cin.eof() && i < rec)
{
cin >> istuid;
if (cin.eof()){i++;continue;}
// Why "continue" for a loop that you would then break out of
// immediately? Also, why are you "counting up a record" when the
// input failed? Also, what do you want to do if the input is
// not a number at all?
else if (!(istuid < 999999 && istuid > 0))
{
cout << "Error, Student ID is invalid\n"
<< "Do you still want to display the result(Y/N)";
/***********************************************
ERROR here
after running this line, it skips the "cin" >_<
***********************************************/

cin >> yn;
if (yn != 'y' || yn != 'Y') // noted logic error here
{
cout << "\nprogram terminates, ";
system("pause");
exit(1);
}
break;
}
cin >> fmark;
// Have you not noticed the duplication here? Think about functions
// in terms of organization, avoiding repetition in the code, and
// representing a logical task - not simply in terms of cutting up
// a long procedure.
if (cin.eof()){i++;continue;}
else if (!(fmark < 100 && fmark > 0))
{
cout << "Error, Student mark is invalid\n"
<< "Do you still want to display the result(Y/N)";
cin >> yn;
if (yn != 'y' || yn != 'Y')
{
cout << "\nprogram terminates, ";
system("pause");
exit(1);
}
break;
}
i++; //i plus plus here, its not showing i dono why
}
}

void vdisplay(int & i)
{
int pert=0;
double average=0,subtra,sd=0,sump=0;
// In C++, you should scope your variables as tightly as possible to avoid
// name collisions; this also means putting them near first use in the same
// scope (since they need not be declared at the top of their scope).
//finding how many records and average
for (int j = 0; j < i; j++)
{
if (fmark[j])
{
pert++;
average += fmark[j];
}
}
if (pert > 0)
{
average = average / pert;
cout << "Pertinent records:\t" << pert <<"\n";
cout << "Mean or average:\t" << average <<"\n";
//end number of records & average

//start loop again for standard deviation
for (int j = 0; j < i; j++)
{
if (fmark[j])
{
subtra = fmark[j] - average;
sump += pow(subtra, 2);
}
}
sd = sump / pert;
sd = sqrt(sd);
cout << "Standard deviation:\t" << sd <<"\n\n";
//end finding standard deviation

//loop last time for displaying records
for (int j = 0; j < i; j++)
{
if (fmark[j])
{
cout << istuid[j] << "\t" <<fmark[j] << "\n";
}
}
}
}



Apparently they still don't teach these things very well in university. Behold:


#include<iostream>
#include<math.h>
#include<vector>
#include<string>
using namespace std;

struct studentRecord {
int id;
float mark;
}; // if you were really lazy, you could make use of std::pair instead

// To avoid extra prototyping effort, I put the helper functions before main.

// Utility to grab a whole line of text from the user (I assume the program will
// only be run interactively) and toss away garbage at the end of the line (so
// that it will not be seen by subsequent reads of cin).
template<typename T>
bool getInput(T& input) {
cin >> input;
// The reading will fail on EOF, invalid input, etc. Check if successful.
bool result = (bool)cin;
// "reset" cin for the next read whether or not this one failed - unless we
// reached eof.
if (!cin.eof()) {
cin.reset();
cin.ignore(numeric_limits<streamsize>::max(), '\n');
}
return result;
}

template<typename T>
bool isValid(T provided, T minimum, T maximum) {
return (provided <= maximum && provided >= minimum);
// I think your version had a logic error too; consider the edge cases.
}

// Give the user the option to terminate the program.
template<typename T>
void promptForTermination(std::string nameOfThing) {
cout << "Error, " << nameOfThing << " is invalid\n"
<< "Do you still want to display the result(Y/N)" << endl;
// The missing "endl" in your code might also be problematic.
std::pair<char, bool> prompt = getInput<char>();
if (prompt != 'y' && prompt != 'Y') {
cout << "\nprogram terminates. "; // fixed comma -> period
system("pause");
exit(1);
}
}

vector<studentRecord> getRecords() {
vector<studentRecords> results;
while (!cin.eof()) {
studentRecord current;
// Try to read studentID; if there's a problem, prompt for termination.
int studentId;
if (getInput(studentId) && isValid(studentId, 1, 999999)) {
// We successfully read a student ID which is in range.
current.id = studentId;
} else {
promptForTermination("Student ID");
continue; // don't ask for a mark for this invalid ID, or store it.
}
// Similarly for the mark. There's still some duplication here. Think about
// what you might do about it.
float studentMark;
if (getInput(studentMark) && isValid(studentMark, 0.0, 100.0)) {
current.mark = studentMark;
} else {
promptForTermination("Student mark");
continue; // don't store this record, because mark is invalid.
}
// We have a valid record (ID and mark).
results.push_back(current);
}
return results;
}

void displayStats(const vector<studentRecord>& data) {
// Count pertinent records and get running sum.
// TODO: don't store pertinent records.
// Is a record with a mark of exactly 0.0 supposed to be valid (though
// ignored for statistics)?
float sum = 0;
int pertinentRecords = 0;
int totalRecords = data.size();
for (int i = 0; i < totalRecords; i++) {
float currentMark = totalRecords.mark;
if (currentMark) {
pertinentRecords++;
sum += currentMark;
}
}
if (!pertinentRecords) return; // no stats.

int average = sum / pertinentRecords;
cout << "Pertinent records:\t" << pertinentRecords << "\n"
<< "Mean or average:\t" << average << endl;

// Dear Professor:
// If you are reading these comments, it indicates that the student has
// blindly copy-and-pasted this code from a message board. If this has
// happened I can only offer you my assurance that it was unintentional on my
// part (I no longer attend any university, much less the student's). The code
// provided was basically intended to illustrate several possible refactorings
// and elements of good C++ style; it is based off of code which he posted
// which was nearly working, but, in my honest opinion, quite awful.
// start loop again for standard deviation
float squaredError;
for (int i = 0; i < totalRecords; i++) {
float currentMark = totalRecords.mark;
if (currentMark) {
float difference = currentMark - average;
squaredError += (difference * difference);
}
}
float rootMeanSquared = sqrt(squaredError / pertinentResults);
cout << "Standard deviation:\t" << rootMeanSquared <<"\n\n";
//end finding standard deviation
// For what it is worth, my experience with learning C++ in university (to
// be fair, it was five or six years ago) was that the curriculum does not
// cover many aspects of modern C++ style - in particular, use of the Standard
// Library - which are essential to writing good C++ code. The original code
// did indeed properly refer to <iostream> instead of the deprecated
// <iostream.h>, and demonstrated awareness of the existence of the std
// namespace, but made no reference to the excellent (and very appropriate
// here) library classes std::vector and std::string.
//loop last time for displaying records
for (int i = 0; i < totalRecords; i++) {
float currentMark = totalRecords.mark;
if (currentMark) {
cout << totalRecords.id << "\t" << currentMark << endl;
}
}
// I therefore exhort you to ensure that this material is covered in this
// student's course, in the future if not now - please, pull any
// administrative strings necessary to ensure this happens. I appeal to your
// interest in the proper education of these young minds - today's university
// students represent the future, as I am sure you are aware (I do not
// actually know what university you are teaching at, but the one I went to
// made a great deal about emphasizing this fact in its PR material).
// In any event, please contact me at karl DOT knechtel AT utoronto DOT ca (I
// retain the email address as an alumnus).
}

int main() {
displayStats(getRecords());
system("pause");
}

Share this post


Link to post
Share on other sites
we are not up to those stuff yet Zahlman
its programing principle 1, and its meant to be a simple c++ program, but thx,
im looking for simpler solutions, actually i want to know whats wrong with my code, and the bug
and.....
Quote:
// Dear Professor:
// If you are reading these comments, it indicates that the student has
// blindly copy-and-pasted this code from a message board. If this has
// happened I can only offer you my assurance that it was unintentional on my
// part (I no longer attend any university, much less the student's). The code
// provided was basically intended to illustrate several possible refactorings
// and elements of good C++ style; it is based off of code which he posted
// which was nearly working, but, in my honest opinion, quite awful.

lol, i will carry your message to my professor

[Edited by - YAITS on May 21, 2005 7:53:38 AM]

Share this post


Link to post
Share on other sites
Quote:
Original post by YAITS
we are not up to those stuff yet Zahlman
its programing principle 1, and its meant to be a simple c++ program, but thx,
im looking for simpler solutions, actually i want to know whats wrong with my code, and the bug
and.....
Quote:
// Dear Professor:
// If you are reading these comments, it indicates that the student has
// blindly copy-and-pasted this code from a message board. If this has
// happened I can only offer you my assurance that it was unintentional on my
// part (I no longer attend any university, much less the student's). The code
// provided was basically intended to illustrate several possible refactorings
// and elements of good C++ style; it is based off of code which he posted
// which was nearly working, but, in my honest opinion, quite awful.

lol, i will carry your message to my professor


ok, we have told you what is wrong. it is not skipping the cin statement but rather your logic is wrong.

as for zahlman - i like your templated methods. personally I would have just made 3 classes - input, affector and data storage.

frankly programming and education is poor. dicrete mathematics is the closest thing you can get in high school. if I was able to teach an 8 year old perl, then I don't think it would be too hard to introduce programming earlier into the curriculum (in tommorows world, how important is this compared with algebra, shakespeare, and all those useless creative arts like pottery and drama).

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!