5
\$\begingroup\$

I have a program to calculate grades that uses pointers and I am trying to figure out if I did it right and if there are any problems with my implementation.

I'm trying to get a more firm grasp on pointers in C++ and have been using them more frequently in my practice programs. I was looking at my grades and thought to myself, "what if I made a program that could calculate grades just like this." Essentially the features it has are: having a Class (not a C++ class, a school class), that has categories. Categories can be things like, "Tests," and, "Classwork," which can be weighted differently. Then you can add assignments into the categories and the grades will be calculated accordingly. You can also change the categories name and weight, as well as attributes of each individual grade. I haven't fully completed the program yet and I plan to redo this code so that it is a little bit less redundant and well, bad; but the features I have seem to work, and my main concern are the pointers and if I am using them correctly/have any problems with them.

One other thing I should mention is that the reason I am using raw pointers is for learning, I know I could probably use something else like smart pointers, or unique pointers.

Class.h:

#include <map>
#include <vector>

class Class{
private:
    struct grade;
    struct category;

    std::vector<category*> categories;
public:
    void addGrade(const std::string&, const std::string&, float, float);
    void addCategory(const std::string&, float);

    void deleteGradeAtIndex(const std::string&, int);
    void deleteGradeByName(const std::string&, const std::string&);
    void deleteCategory(const std::string&);

    void changeGradeAtIndex(const std::string&, int, const std::string&, float, float);
    void changeGradeByName(const std::string&, const std::string&, const std::string&, float, float);
    void changeCategory(const std::string&, const std::string&, float);

    void printGrades();
};

Class.cpp:

#include <iostream>
#include <string>
#include "Class.h"

struct Class::category{
    float weight;
    float percentage;
    std::string name;
    std::vector<grade*> grades;
};

struct Class::grade{
    float ptsEarned;
    float ptsPossible;
    float percentage;
    std::string name;
};

void Class::addCategory(const std::string& name, float weight){
    category* temp = new category;

    temp->weight = weight;
    temp->name = name;

    categories.push_back(temp);
}

void Class::addGrade(const std::string& category, const std::string& name, float pEarn, float pPoss){
    for(uint32_t i = 0; i < categories.size(); i++){
        if(categories[i]->name.compare(category) == 0){
            grade* temp = new grade;

            temp->name = name;
            temp->ptsEarned = pEarn;
            temp->ptsPossible = pPoss;
            temp->percentage = (pEarn/pPoss)*100;

            categories[i]->grades.push_back(temp);

            float pos = 0, ear = 0;

            for(uint32_t j = 0; j < categories[i]->grades.size(); j++){
                pos += categories[i]->grades[j]->ptsPossible;
                ear += categories[i]->grades[j]->ptsEarned;
            }
            categories[i]->percentage = (ear/pos)*100;

            return;
        }
    }

    std::cout << "category not found" << std::endl;
}

void Class::deleteGradeAtIndex(const std::string& category, int index){
    for(uint32_t i = 0; i < categories.size(); i++){
        if(categories[i]->name.compare(category) == 0){
            delete categories[i]->grades[index];
            categories[i]->grades[index] = nullptr;

            categories[i]->grades.erase(categories[i]->grades.begin() + index);
            return;
        }
    }
}

void Class::deleteGradeByName(const std::string& category, const std::string& gradeName){
    for(uint32_t i = 0; i < categories.size(); i++){
        if(categories[i]->name.compare(category) == 0){
            for(uint32_t j = 0; j < categories[i]->grades.size(); j++){
                if(categories[i]->grades[j]->name.compare(gradeName) == 0){
                    delete categories[i]->grades[j];
                    categories[i]->grades[j] = nullptr;

                    categories[i]->grades.erase(categories[i]->grades.begin() + j);
                    return;
                }
            }
        }
    }

}
void Class::deleteCategory(const std::string& category){
    for(uint32_t i = 0; i < categories.size(); i++){
        if(categories[i]->name.compare(category) == 0){
            for(uint32_t j = 0; j < categories[i]->grades.size(); j++){
                delete categories[i]->grades[j];
                categories[i]->grades[j] = nullptr;

                categories[i]->grades.erase(categories[i]->grades.begin() + j);
            }
            delete categories[i];
            categories[i] = nullptr;

            categories.erase(categories.begin() + i);
            return;
        }
    }
}

void Class::changeCategory(const std::string& category, const std::string& newName, float newWeight){
    for(uint32_t i = 0; i < categories.size(); i++){
        if(categories[i]->name.compare(category) == 0){
            categories[i]->name = newName;
            categories[i]->weight = newWeight;
            return;
        }
    }
}

void Class::changeGradeAtIndex(const std::string& category, int index, const std::string& newName, float pEarn, float pPoss){
    for(uint32_t i = 0; i < categories.size(); i++){
        if(categories[i]->name.compare(category) == 0){
            categories[i]->grades[index]->name = newName;
            categories[i]->grades[index]->ptsEarned = pEarn;
            categories[i]->grades[index]->ptsPossible = pPoss;
            categories[i]->grades[index]->percentage = (pEarn/pPoss)*100;

            float pos = 0, ear = 0;

            for(uint32_t j = 0; j < categories[i]->grades.size(); j++){
                pos += categories[i]->grades[j]->ptsPossible;
                ear += categories[i]->grades[j]->ptsEarned;
            }
            categories[i]->percentage = (ear/pos)*100;

            return;
        }
    }
}

void Class::changeGradeByName(const std::string& category, const std::string& gradeName, const std::string& newName, float pEarn, float pPoss){
    for(uint32_t i = 0; i < categories.size(); i++){
        if(categories[i]->name.compare(category) == 0){
            for(uint32_t j = 0; j < categories[i]->grades.size(); j++){
                if(categories[i]->grades[j]->name.compare(gradeName) == 0){
                    categories[i]->grades[j]->name = newName;
                    categories[i]->grades[j]->ptsEarned = pEarn;
                    categories[i]->grades[j]->ptsPossible = pPoss;
                    categories[i]->grades[j]->percentage = (pEarn/pPoss)*100;

                    float pos = 0, ear = 0;

                    for(uint32_t j = 0; j < categories[i]->grades.size(); j++){
                        pos += categories[i]->grades[j]->ptsPossible;
                        ear += categories[i]->grades[j]->ptsEarned;
                    }
                    categories[i]->percentage = (ear/pos)*100;

                    return;
                }
            }
        }
    }
}

void Class::printGrades(){
    for(uint32_t i = 0; i < categories.size(); i++){
        std::cout << "Category: " << categories[i]->name << " | weight: " << categories[i]->weight << " | " << categories[i]->percentage << "%\n";

        for(uint32_t j = 0; j < categories[i]->grades.size(); j++){
            std::cout << "\tassignment: " << categories[i]->grades[j]->name
                 << " | points earned: " << categories[i]->grades[j]->ptsEarned
                 << " | points possible: " << categories[i]->grades[j]->ptsPossible
                 << " | " << categories[i]->grades[j]->percentage << "%" << std::endl;
        }
    }
}

main.cpp:

int main()
{
    Class math;
    math.addCategory("tests", 70);
    math.addGrade("tests", "test1", 95, 100);
    math.addGrade("tests", "test2", 80, 100);
    math.addCategory("class work", 30);
    math.addGrade("class work", "assignment1", 100, 100);
    math.addGrade("class work", "assignment2", 100, 100);

    math.deleteGradeByName("class work", "assignment1");

    math.changeGradeAtIndex("tests", 0, "test1c", 93, 100);
    math.changeGradeByName("tests", "test2", "test2c", 95, 100);

    math.printGrades();
}
\$\endgroup\$
2
  • \$\begingroup\$ I know I could probably use something else like smart pointers, or unique pointers. Or no pointers at all ;) \$\endgroup\$
    – Rakete1111
    Commented Nov 25, 2018 at 12:14
  • \$\begingroup\$ By the way, Class is a horrible name for a class. Could you think of something more descriptive? What is the class actually doing? \$\endgroup\$
    – Juho
    Commented Nov 25, 2018 at 14:14

1 Answer 1

Reset to default
4
\$\begingroup\$

Welcome on Code Review!

Congratulation, it's a pretty clean code, just some points:

  • There's missing the #include "Class.h" in main.cpp
  • Although a C string literal can be implicitly converted to a std::string, if you don't #include <string> in main.cpp Clang with the -pedantic flag will complain.
  • In Class::changeGradeByName you hide the previous declared j in your 3rd-nested for-loop.
  • Since the header file is what the user (or you, later) will look at first to understand your interface, a good practice is to don't omit parameter name in prototypes. It make the interface more explicit:

Compare:

void changeGradeByName(const std::string&, const std::string&, const std::string&, float, float)

Versus:

void changeGradeByName(const std::string& category, const std::string& gradeName, const std::string& newName, float pEarn, float pPoss);
  • Use meaningful names. In this latter, what mean pPoss or pEarn ?
  • You should provide constructor/destructor for your structs, it will make the code easier.

I surely miss something, but I hope others will come and review your code with a different perspective or another angle.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks this was really helpful, and I've been trying to work on making my code cleaner. The J in the third for loop in Class::changeGradeByName was a mistake I didn't notice, thanks for pointing that out, i didn't even know something like that would compile. I added a constructor and a destructor to my structs which also shortened it a bit. #include "Class.h" in main.cpp is there, I just accidentally left that bit off when I copied main.cpp. Also i will take note to put parameter identifiers in my header too because it does get confusing when reading the header before reading the source. \$\endgroup\$
    – KevinCMD
    Commented Nov 25, 2018 at 2:00
  • \$\begingroup\$ And didn't forget to add #include <string> in main.cpp otherwise Clang will complain with the -pedantic flag. (Or 1// provide const char* overloads, or 2// use c++14 and string literal operator to defines your string, or 3// explicitly pass std::string("tests") to your functions. But I think the include is the simplest. ;)) \$\endgroup\$
    – Calak
    Commented Nov 25, 2018 at 2:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.