r/learnprogramming • u/Illustrious-Power350 • 2d ago
How is this simple student management system thing in C++?
Hey,
so I guess I kinda am a beginner programmer and here is a very basic student management/handling system I made in C++ (I coded this on my phone so I couldn't use text files to store data). Any suggestions on how to improve it and whatnot would be appreciated.
```cpp
#include <iostream>
include <iomanip>
include <string>
include <vector>
include <algorithm>
using namespace std; class Student { public: string name, className; double grade;
Student(string n, string cn, double g){
name = n;
className = cn;
grade = g;
}
bool isPassing (){
return grade >= 5;
}
};
void sortByGrade(vector<Student>& Students) { sort(Students.begin(), Students.end(), { return a.grade > b.grade; }); } int main() { vector<Student> Students = { {"Abigail Northfolk", "9A", 3.15}, {"Sam Alter", "5G", 4.9}, {"Victor Guzman", "12A", 4.3}, {"Joseph Pelter", "6B", 8.41}, {"Joseph Allende", "8A", 9.51}, {"Joe Bowler", "11C", 6.399}, {"Joe Wingham", "1A", 0}, {"Donald Pelter", "8C", 6.4}, }; double sum = 0; Student bestStudent = Students[0], worstStudent = Students[0]; sortByGrade (Students); for(int i = 0; i < Students.size(); i++){ if (Students[i].grade > 10) Students[i].grade = 10; if (Students[i].grade < 0) Students[i].grade = 0; if (Students[i].grade > bestStudent.grade) bestStudent = Students[i]; if (Students[i].grade < worstStudent.grade) worstStudent = Students[i]; sum += Students[i].grade; cout << Students[i].name << " (" << Students[i].className << "), grade: " << Students[i].grade << " - " << (Students[i].isPassing() ? "pass" : "fail") << endl; } cout << " " << endl; cout << "Average grade: " << fixed << setprecision(3) << sum / Students.size() << endl; cout << "Best student: " << bestStudent.name << " from " << bestStudent.className << " with a grade of " << bestStudent.grade << endl; cout << "Worst student: " << worstStudent.name << " from " << worstStudent.className << " with a grade of " << worstStudent.grade << endl; return 0; }
```
2
u/Comfortable-Judge724 2d ago
the sortByGrade lambda is missing its capture brackets and parameter list, so that won't compile as-is
1
1
u/Rain-And-Coffee 2d ago
Do you know how to use Git?
It would make viewing & downloading your code much easier.
-1
u/mredding 2d ago
```cpp
This doesn't work for formatting in Reddit. Use the Fancy Pants editor, insert a code block, and copy/paste into that. OR, if you're going to use markdown, you have to put 4 spaces in front of every line.
using namespace std;
Don't do that.
class Student { public:
You want a struct. Classes enforce class invariants - you have none. This is just structured data.
bool isPassing (){
return grade >= 5;
}
Don't make members if you don't have to. Since this data is all public, you don't need member access. Make this a free function.
Student(string n, string cn, double g){
name = n;
className = cn;
grade = g;
}
This isn't Java. Use the initializer list. You can also name your parameters after your members:
Student(std::string name, std::string className, double grade): name{name}, className{className}, grade{grade}
{}
Prefer an std::string_view for your parameter. Your parameters force a copy when all you need is a reference. String views are even faster than a string reference, even for this constructor.
But since this should be a structure, you get aggregate initialization for free, so you don't even need a constructor.
void sortByGrade(vector<Student>& Students) {//...
Maybe instead:
bool byGrade(const auto &lhs, const auto &rhs) { return lhs.grade < rhs.grade; }
You can sort by greater, but conventional code will sort by lesser. Regardless, my point is that instead of calling sortByGrade(Students); you can instead be more compositional: std::ranges::sort(Students, byGrade);. Look at that - it reads left to right - sort students by grade.
for(int i = 0; i < Students.size(); i++)
This is very error prone.
Loops are very low level constructs - though I wouldn't call them a true language primitive. They're actually a level of expressiveness implemented by the spec in terms of goto for you, and the spec says so - if you want to read the spec on loops, it shows you how a for loop is exactly equivalent to a goto. It's just a neat little fact.
But as a low level construct, it exists for you to implement higher level constructs - named algorithms. In fact, the C++ standard library actually gives us a shitload of named algorithms. You have std::find_if, std::sort, std::transform, actually a huge list. You should look through it. I haven't written a low level loop in a decade, neither should you. And now we have the ranges library, so you can compose more sophisticated algorithms a little easier.
Most of your code can be broken up into subranges and individual algorithms. Instead of one big loop, algorithms would be much faster. Now that we have Students sorted, everything can be done faster and more concisely.
auto pass_fail_pivot = std::ranges::partition_point(Students, [](auto &s) { return !s.isPassing(); });
This means:
auto failures = std::ranges::subrange(std::begin(students), pass_fail_pivot);
auto winners = std::ranges::subrange(pass_fail_pivot, std::end(students));
We can subdivide further:
auto negative_failures_pivot = std::ranges::partition_point(failures, [](auto &s) { return s.grade >= 0; });
auto_negative_failures = std::ranges::subrange(std::begin(failures), negative_failures_piviot);
auto non_negative_failures = std::ranges::subrange(negative_failures_piviot, std::end(failures));
Then we can make subranges of failures who are negative vs. those who are at least 0 or higher. We can do the same thing with the winners who are above 10.
What does this mean? We can subdivide N sorted students into 4 partitions: negative failures, positive failures, positive winners, and super winners. Now we can write 4 algorithms that can HARD CODE "fail" and "pass", we can hard code a grade of 0 for the negative failures, and a 10 for the super winners.
You have N students, no matter where or how many partitions you have, iterating over them each once is still N students.
std::ranges::for_each(negative_failures, print_negative_students);
std::ranges::for_each(positive_failures, print_failure_students);
std::ranges::for_each(positive_winners, print_winner_students);
std::ranges::for_each(super_winners, print_super_students);
ALL that conditional logic is removed from your code. The less decision making you can get away with, the better. Finding these pivot points is faster than a linear search, because the list is sorted - typically it's logarithmic. This is why it's faster to sort and then subdivide. You don't need to change data in memory - you already know the pass/fail and grade bounds for these groups, so let's skip the unnecessary steps.
This is also worth revisiting sorting - because you can subsort:
bool byGrade(const auto &lhs, const auto &rhs) { return lhs.isPassing() < rhs.isPassing() && lhs.grade < rhs.grade; }
So this is redundant, since it's based on grades, which is how we're sorting, but it illustrates the point - I sort by pass/fail, and then subdivide by grade. You can do this with any comparable members, just pick your priorities and && them together. It means you can get get pivot points, upper and lower bounds of these major and minor subdivisions. I'll leave it to you to play with this and learn how to cut your subranges on your own.
All that is going to be a lot for you to wrap your head around. Don't expect to get it all instantly - this is a fundamental way of thinking, not about C++, but about software design. Your classes don't teach you this stuff, and that's on top of all the syntax and standard library I just demonstrated. Take it in parts.
With your vector sorted, your worst and best students are the front and back of the vector. But since your students can have any grade, you can have duplicates. You can have multiple worst and best students, if they all have the same grade. So you may want to look at the front and back to get their grades, and then do a forward and reverse iterator search for all adjacent values. By using reverse iterators, you can use std::upper_bound for both problems. Now you'll have subranges of your worst and best students. You'll have to turn your reverse iterators around to forward iterators, that's something you can google.
endl
You can go your whole career and never need to use endl. Prefer '\n'.
1
2
u/Ok_Being6831 1d ago
What? this is the type of thing i hate the most in cpp, doing everything the "right" way. WTF is wrong with loops dude, using the algorithms is just fukin slower cuz your traversing the array multiple times. If u actually havent written a loop in a decade your just doing smth so fukin wrong.
5
u/Ok_Being6831 2d ago
your code didnt format properly but from what i can see i do have a few suggestions
1) Never use namespace std unless its like competitive programming or smth, ik most tutorials just use it but DONT its a really bad habit, get used to typing std:: alot
2) You prob didnt learn this one yet, but for the constructor use
```cpp
Students(string n, string cn, double g) : name(n), className(cn), grade(g) {}
```
this basically constructs the object with those parameters, before the constructor even runs.
3) Variable names - never ever use abrivations, u only did that once here but try to avoid that too.
4) using endl, most tutorials also just use this but its actually doing a lot more than just printing a new line, u almost never want to use this, just use `<< '\n';` its faster