Simple GradeSystem

[Zeveso]

Hello,
New here and hope to stay. Anyways...

I have started back with C++ and wanted to see how much I remembered. So please tell me what I am doing wrong. I am on linux at the moment and could not remeber if you used <iostream> with windows or linux.

Also need explanations on what do, I believe one of the main problems with my code is I need to keep the variables in ( ).

Code:

#include <iostream>
#include <fstream> // open, read, and write files

//for time
#include <stdio.h>
#include <stdlib.h>
#include <time.h>

using namespace std;

void writeGrade(int studentAverage, int letterGrade){
/*
Use the variable gradeAverage to determin the the grade was a A, B, C, D, or F. Then open of the person's file, and write the grade.
*/
findLetter();
ofstream gradeFile;
gradeFile.open ("grades.txt");
gradeFile << "Student: " << studentAverage << endl;
gradeFile << "Grade: " << letterGrade << endl;
gradeFile << "Average: " << studentAverage << endl << endl;
}

int theAverage(int totalAmount, int gradeNumber, int studentAverage){
/*
Here we will average all the numbers together inside of a for loop
*/
totalAmount / gradeNumber == studentAverage;
return studentAverage;
}

int findLetter(){
//take the number and find the letter it is == to
//then store that letter in letterGrade
if ( studentAverage > 95 ){
letterGrade == A;
}
if ( studentAverage > 85 ){
letterGrade == B;
}
if ( studentAverage > 80 ){
letterGrade == C;
}
if ( studentAverage > 75 ){
letterGrade == D;
}
if ( studentAverage < 69 ){
letterGrade == F;
}
return letterGrade;
}

void saveError(){
// save error in log.txt
nowTime(int theTime);
ofstream logFile;
logFile.open ("log.txt");
logFile << "Date: " << theTime << endl;
logFile << "Error: File does not exist or cannot be opened." << endl;
logFile << "Please create file, or create a new file." << endl << endl;
}

int nowTime(){

time_t now;
time(&now);

theTime = "%s", ctime(&now);

return theTime;
}

int main(){
//startup variables that we will use
char studentName[255];
int studentAverage;
int gradeNumber; // amount of student Averages
int totalAmount; //will hold all averages
int submitAmount; //amount submited during for loop session
char letterGrade; // will store the lettergrade before it is written to grades.txt
int theTime; // will hold the current time
bool canWe; // asks if we can continue


// find out who we will be grading
cout << "Hello user!" << endl;
cout << "This program lets you average the grade of your student and write there grade in a text file." << endl;
cout << "Who will be writting a grade for today?" << endl << "Name: ";
cin >> studentName >> endl;
cout << "Ok, we will be grading " << studentName << "today!" << endl;

// make a loop for finding out the average
for ( gradeNumber; canWe == true; gradeNumber++; ){
cout << "Please submit the next grade." << endl << "Grade: ";
cin >> submitAmount;
submitAmount + totalAmount = totalAmount;
cout << "Thank you, the grade has been added." << endl;
cout << "Would you like to add another grade or would you like to quite?" << endl;
cout << "Type 1 to contine or type 2 to quite and press <enter>." << endl << "1 or 0: "; // may need to be switched to true or false
cin >> canWe >> endl;
return totalAmount, gradeNumber;
}

theAverage();
writeGrade();


return 0;
}

[GCDEF]

I didn't look at the whole thing, but let's just look at this.

if ( studentAverage > 95 ){
letterGrade == A;
}
if ( studentAverage > 85 ){
letterGrade == B;
}
if ( studentAverage > 80 ){
letterGrade == C;
}
if ( studentAverage > 75 ){
letterGrade == D;
}
if ( studentAverage < 69 ){
letterGrade == F;
}

Assume studentAverage is 96.

The first test is true, so letterGrade gets assigned A.
The second test is also true, so letterGrade no gets assigned a B, etc.

letterGrade is defined in two places, one as an int and one as a char. A, B, C, etc. aren't defined anywhere. If you mean to use them as chars, wrap them in single quotes.

[monarch_dodra]

I didn't look at the whole thing, but let's just look at this.

if ( studentAverage > 95 ){
letterGrade == A;
}
if ( studentAverage > 85 ){
letterGrade == B;
}
if ( studentAverage > 80 ){
letterGrade == C;
}
if ( studentAverage > 75 ){
letterGrade == D;
}
if ( studentAverage < 69 ){
letterGrade == F;
}

Assume studentAverage is 96.

The first test is true, so letterGrade gets assigned A.
The second test is also true, so letterGrade no gets assigned a B, etc.

letterGrade is defined in two places, one as an int and one as a char. A, B, C, etc. aren't defined anywhere. If you mean to use them as chars, wrap them in single quotes.

May I add that none of those are actually assignements? They are all just equality operators, and hence, do nothing. You want "=" not "=="

...

The funny part is that people usually mess up the other way:

if (i = 5) //always true
{
    ...
}

[MrViggy]

And, remove one of the '=' signs:

if ( studentAverage > 95 ){
letterGrade = 'A';
}

Viggy

[Lindley]

totalAmount / gradeNumber == studentAverage;

I think what you meant was

studentAverage = totalAmount / gradeNumber;

Assignment always goes right-to-left. Note also that every variable here is an integer, which might not be what you want.

[Zeveso]

thank you all so far, made changes

Errors:

root@bt:~/gradesystem# make gradeSystem
g++     gradeSystem.cpp   -o gradeSystem
gradeSystem.cpp: In function 'void writeGrade(int, int)':
gradeSystem.cpp:17: error: 'findLetter' was not declared in this scope
gradeSystem.cpp: In function 'int findLetter()':
gradeSystem.cpp:36: error: 'studentAverage' was not declared in this scope
gradeSystem.cpp:37: error: 'letterGrade' was not declared in this scope
gradeSystem.cpp:37: error: 'A' was not declared in this scope
gradeSystem.cpp:39: error: 'studentAverage' was not declared in this scope
gradeSystem.cpp:40: error: 'letterGrade' was not declared in this scope
gradeSystem.cpp:40: error: 'B' was not declared in this scope
gradeSystem.cpp:42: error: 'studentAverage' was not declared in this scope
gradeSystem.cpp:43: error: 'letterGrade' was not declared in this scope
gradeSystem.cpp:43: error: 'C' was not declared in this scope
gradeSystem.cpp:45: error: 'studentAverage' was not declared in this scope
gradeSystem.cpp:46: error: 'letterGrade' was not declared in this scope
gradeSystem.cpp:46: error: 'D' was not declared in this scope
gradeSystem.cpp:48: error: 'studentAverage' was not declared in this scope
gradeSystem.cpp:49: error: 'letterGrade' was not declared in this scope
gradeSystem.cpp:49: error: 'F' was not declared in this scope
gradeSystem.cpp:51: error: 'letterGrade' was not declared in this scope
gradeSystem.cpp: In function 'void saveError()':
gradeSystem.cpp:56: error: expected primary-expression before 'int'
gradeSystem.cpp:56: error: 'nowTime' was not declared in this scope
gradeSystem.cpp:59: error: 'theTime' was not declared in this scope
gradeSystem.cpp: In function 'int nowTime()':
gradeSystem.cpp:69: error: 'theTime' was not declared in this scope
gradeSystem.cpp: In function 'int main()':
gradeSystem.cpp:90: error: no match for 'operator>>' in 'std::operator>> [with _CharT2 = char, _Traits2 = std::char_traits<char>, _CharT = char, _Traits = std::char_traits<char>](((std::basic_istream<char, std::char_traits<char> >&)(& std::cin)), ((char*)(& studentName))) >> std::endl'
/usr/include/c++/4.3/istream:123: note: candidates are: std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(std::basic_istream<_CharT, _Traits>& (*)(std::basic_istream<_CharT, _Traits>&)) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:127: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(std::basic_ios<_CharT, _Traits>& (*)(std::basic_ios<_CharT, _Traits>&)) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:134: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(std::ios_base& (*)(std::ios_base&)) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:170: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(bool&) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:174: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(short int&) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:177: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(short unsigned int&) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:181: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(int&) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:184: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(unsigned int&) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:188: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(long int&) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:192: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(long unsigned int&) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:197: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(long long int&) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:201: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(long long unsigned int&) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:206: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(float&) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:210: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(double&) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:214: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(long double&) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:218: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(void*&) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:242: note:                 std::basic_istream<_CharT, _Traits>& std::basic_istream<_CharT, _Traits>::operator>>(std::basic_streambuf<_CharT, _Traits>*) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/istream:748: note:                 std::basic_istream<_CharT2, _Traits2>& std::operator>>(std::basic_istream<_CharT2, _Traits2>&, _CharT2*) [with _CharT2 = char, _Traits2 = std::char_traits<char>, _CharT = char, _Traits = std::char_traits<char>]
/usr/include/c++/4.3/bits/istream.tcc:858: note:                 std::basic_istream<_CharT, _Traits>& std::operator>>(std::basic_istream<_CharT, _Traits>&, _CharT&) [with _CharT = char, _Traits = std::char_traits<char>]
gradeSystem.cpp:94: error: expected `)' before ';' token
gradeSystem.cpp:94: error: expected primary-expression before ')' token
gradeSystem.cpp:94: error: expected `;' before ')' token
gradeSystem.cpp:25: error: too few arguments to function 'int theAverage(int, int, int)'
gradeSystem.cpp:105: error: at this point in file
gradeSystem.cpp:13: error: too few arguments to function 'void writeGrade(int, int)'
gradeSystem.cpp:106: error: at this point in file
make: *** [gradeSystem] Error 1

here is the new code:

#include <iostream>
#include <fstream> // open, read, and write files

//for time
#include <stdio.h>
#include <stdlib.h>
#include <time.h>

using namespace std;

void writeGrade(int studentAverage, int letterGrade){
/*
Use the variable gradeAverage to determin the the grade was a A, B, C, D, or F. Then open of the person's file, and write the grade.
*/
  findLetter();
  ofstream gradeFile;
  gradeFile.open ("grades.txt");
  gradeFile << "Student: " << studentAverage << endl;
  gradeFile << "Grade: " << letterGrade << endl;
  gradeFile << "Average: " << studentAverage << endl << endl;
}

int theAverage(int totalAmount, int gradeNumber, int studentAverage){
/*
Here we will average all the numbers together inside of a for loop
*/
  studentAverage = totalAmount / gradeNumber;
  return studentAverage;
}

int findLetter(){
//take the number and find the letter it is == to
//then store that letter in letterGrade
  if ( studentAverage > 95 ){
  letterGrade = A;
  }
  if ( 95 > studentAverage > 85 ){
  letterGrade = B;
  }
  if ( 85 > studentAverage > 80 ){
  letterGrade = C;
  }
  if ( 80 > studentAverage > 69 ){
  letterGrade = D;
  }
  if ( studentAverage < 69 ){
  letterGrade = F;
  }
  return letterGrade;
}

void saveError(){
// save error in log.txt
  nowTime(int theTime);
  ofstream logFile;
  logFile.open ("log.txt");
  logFile << "Date: " << theTime << endl;
  logFile << "Error: File does not exist or cannot be opened." << endl;
  logFile << "Please create file, or create a new file." << endl << endl;
}

int nowTime(){

  time_t now;
  time(&now);

  theTime = "%s", ctime(&now);

return theTime;
}

int main(){
  //startup variables that we will use
  char studentName[255];
  int studentAverage;
  int gradeNumber; // amount of student Averages
  int totalAmount; //will hold all averages
  int submitAmount; //amount submited during for loop session
  char letterGrade; // will store the lettergrade before it is written to grades.txt
  int theTime; // will hold the current time
  bool canWe; // asks if we can continue
  

  // find out who we will be grading
  cout << "Hello user!" << endl;
  cout << "This program lets you average the grade of your student and write there grade in a text file." << endl;
  cout << "Who will be writting a grade for today?" << endl << "Name: ";
  cin >> studentName >> endl;
  cout << "Ok, we will be grading " << studentName << "today!" << endl;

  // make a loop for finding out the average
  for ( gradeNumber; canWe == true; gradeNumber++; ){
  cout << "Please submit the next grade." << endl << "Grade: ";
  cin >> submitAmount;
  submitAmount + totalAmount = totalAmount;
  cout << "Thank you, the grade has been added." << endl;
  cout << "Would you like to add another grade or would you like to quite?" << endl;
  cout << "Type 1 to contine or type 2 to quite and press <enter>." << endl << "1 or 0: "; // may need to be switched to true or false
  cin >> canWe >> endl;
  return totalAmount, gradeNumber;
  }

  theAverage();
  writeGrade();
  

return 0;
}

Will be working though this myself to see what I can do, but any help is appreciated.

[GCDEF]

if ( studentAverage > 95 ){
letterGrade = A;
}
if ( 95 > studentAverage > 85 ){
letterGrade = B;
}
if ( 85 > studentAverage > 80 ){
letterGrade = C;
}
if ( 80 > studentAverage > 69 ){
letterGrade = D;
}
if ( studentAverage < 69 ){
letterGrade = F;
}
return letterGrade;
}

That still isn't optimal. You were closer the first time. Just use else between the if statements so that when one matches, the others are bypassed.

[itsmeandnobodyelse]

Here an implementation of the findLetter which doesn't use an if/elseif cascade.

char findLetter(int grade)
{
    // put the ranges to an array
    static int grades[] = { 69, 75, 80, 85, 90, 95 };

    // init return value
    char letter = 'A';
    // get number of elements
    int nGrades = sizeof(grades)/sizeof(grades[0]);

    // now check the ranges by a loop 
    for (int i = 0; i < nGrades; ++i)
    {
        // if the grade given is below we are done
        if (grade <= grades[i]) 
        {
            // grade character can be calculated from index 
            letter = 'A' + (char)(nGrades-1 - i);  // F for i==0, E for i==1, ...
            break;
        }
    }
    return letter;
}

Regards, Alex

[Zeveso]

Here an implementation of the findLetter which doesn't use an if/elseif cascade.

char findLetter(int grade)
{
    // put the ranges to an array
    static int grades[] = { 69, 75, 80, 85, 90, 95 };

    // init return value
    char letter = 'A';
    // get number of elements
    int nGrades = sizeof(grades)/sizeof(grades[0]);

    // now check the ranges by a loop 
    for (int i = 0; i < nGrades; ++i)
    {
        // if the grade given is below we are done
        if (grade <= grades[i]) 
        {
            // grade character can be calculated from index 
            letter = 'A' + (char)(nGrades-1 - i);  // F for i==0, E for i==1, ...
            break;
        }
    }
    return letter;
}

Regards, Alex

I am gonna be honnest with you, that makes no sense... i don't see how that would even work... much less help me

[GCDEF]

Here an implementation of the findLetter which doesn't use an if/elseif cascade.

char findLetter(int grade)
{
    // put the ranges to an array
    static int grades[] = { 69, 75, 80, 85, 90, 95 };

    // init return value
    char letter = 'A';
    // get number of elements
    int nGrades = sizeof(grades)/sizeof(grades[0]);

    // now check the ranges by a loop 
    for (int i = 0; i < nGrades; ++i)
    {
        // if the grade given is below we are done
        if (grade <= grades[i]) 
        {
            // grade character can be calculated from index 
            letter = 'A' + (char)(nGrades-1 - i);  // F for i==0, E for i==1, ...
            break;
        }
    }
    return letter;
}

Regards, Alex

Not sure I really see the advantage to that. It doesn't save any lines of code, isn't any more efficient and is less readable.

[itsmeandnobodyelse]

Not sure I really see the advantage to that. It doesn't save any lines of code, isn't any more efficient and is less readable.

The advantages of that compared to an if/else if cascade is that the relevant information is given in one single array. If you want to change the number of grades or the ranges for a single grade you have to change that array and nothing else.

The code has 8 statements (brackets not counted). The original code has 11 statements. So the assertment 'it doesn't save any lines of code' is wrong. Efficiency IMO is not the main goal of such a function but surely a loop is a far better coding technique than repeating similar statements multiple times. I also think that packing all relevant information to an array improves readability. Maybe the following code would point out that aspect more clearly:

char findLetter(int grade)
{
    struct Grade
    {
        int low;
        int high;
        char grade;
    };

    enum { LOW = 0, HIGH = 120 };
    static Grade grades[] = 
    { 
        { LOW, 69,   'F', },
        {  70,   75,   'E', },
        {  76,   80,   'D', },
        {  81,   85,   'C', },
        {  86,   95,   'B', },
        {  96, HIGH, 'A', },
    };

    char letter = '?';
    int  nGrades = sizeof(grades)/sizeof(grades[0]);
    for (int i = 0; i < nGrades; ++i)
    {
        if (grade >= grades[i].low && grade <= grades[i].high) 
        {
            letter = grades[i].grade;
            break;
        }
    }
    return letter;
}

Though now it is more lines, less efficient, it is well readable, less error-prone and good quality (and tested).

Regards, Alex

[monarch_dodra]

The advantages of that compared to an if/else if cascade is that the relevant information is given in one single array. If you want to change the number of grades or the ranges for a single grade you have to change that array and nothing else.

The code has 8 statements (brackets not counted). The original code has 11 statements. So the assertment 'it doesn't save any lines of code' is wrong. Efficiency IMO is not the main goal of such a function but surely a loop is a far better coding technique than repeating similar statements multiple times. I also think that packing all relevant information to an array improves readability. Maybe the following code would point out that aspect more clearly:

char findLetter(int grade)
{
    struct Grade
    {
        int low;
        int high;
        char grade;
    };

    enum { LOW = 0, HIGH = 120 };
    static Grade grades[] = 
    { 
        { LOW, 69,   'F', },
        {  70,   75,   'E', },
        {  76,   80,   'D', },
        {  81,   85,   'C', },
        {  86,   95,   'B', },
        {  96, HIGH, 'A', },
    };

    char letter = '?';
    int  nGrades = sizeof(grades)/sizeof(grades[0]);
    for (int i = 0; i < nGrades; ++i)
    {
        if (grade >= grades[i].low && grade <= grades[i].high) 
        {
            letter = grades[i].grade;
            break;
        }
    }
    return letter;
}

Though now it is more lines, less efficient, it is well readable, less error-prone and good quality (and tested).

Regards, Alex

While I see what you are getting at, It seems un-necessarily complicated. I'm fairly certain a standard array + binary_search would be simpler/easier to implement/safer/easier to maintain/easier to understand/faster.

Or heck, a (static const) map, and lower_bound. This solution is not only fast, it is very easy to implement, and very easy to understand.

But now I think we are staying from the subject

[VladimirF]

...The code has 8 statements (brackets not counted). The original code has 11 statements. So the assertment 'it doesn't save any lines of code' is wrong. Efficiency IMO is not the main goal of such a function but surely a loop is a far better coding technique than repeating similar statements multiple times. I also think that packing all relevant information to an array improves readability...

Consider this:

char findLetter(int grade)
{
	static char LetterGrade[] = "F...FD...DC...CB...BA...A";
	return LetterGrade[grade];
}

The ellipsis, of course, must be replaced with appropriate number of F’s, D’s, etc.

[Lindley]

Consider this:

char findLetter(int grade)
{
	static char LetterGrade[] = "F...FD...DC...CB...BA...A";
	return LetterGrade[grade];
}

The ellipsis, of course, must be replaced with appropriate number of F’s, D’s, etc.

Good idea. You could further improve it by, instead of requiring one to manually count letters when creating the char array, instead populating a vector<char> using Boost.Assign's repeat() function.

[Paul McKenzie]

Hello,
New here and hope to stay. Anyways...

What grade does the student get if he gets a 70? Your attempt does not show what happens if the grade is between 70 and 75.

Regards,

Paul McKenzie