class pointer unitialized local variable

Question

Recently in my c++ class we have learned about pointers and classes.

I'm trying to make a program that has a class Student, which we will point to give each student a name and test score.

After entering both name and test score, they are sorted and then listed in order of highest to lowest.

I believe all my syntax to be correct, however I am still learning. The problem I am having is that the first time I use my class I get an uninitialized local variable error, any help on how to fix this?

#include "stdafx.h"
#include <iostream>
#include <string>
#include <array>
using namespace std;

class Student {
private:
    double score;
    string name;
public:
    void setScore(double a) {
        score = a;
    }
    double getScore() {
        return score;
    }
    void setName(string b) {
        name = b;
    }
    string getName() {
        return name;
    }
};

void sorting(Student*, int);

int main()
{
    Student *students;
    string name;
    int score;
    int *count;
    count = new int;
    cout << "How many students? ";
    cin >> *count;
    while (*count <= 0) {
        cout << "ERROR: The number of students must be greater than 0.\n";
        cin >> *count;
    }
    for (int i = 0; i < *count; i++) {
        cout << "Please enter the students name: ";
        cin >> name;
        students[i].setName(name);
        cout << "Please enter " << students[i].getName() << "'s score: ";
        cin >> score;
        while (score < 0) {
            cout << "ERROR: Score must be a positive number.\n";
            cin >> score;
        }
        students[i].setScore(score);
    }
    sorting(students, *count);
    for (int i = 0; i < *count; i++) {
        cout << students[i].getName() << ": " << students[i].getScore() << endl;
    }
    system("PAUSE");
    return 0;
}

void sorting(Student *s, int size) {
    for (int i = 0; i < size; i++) {
        for (int j = i; j < size; j++) {
            if (s[j].getScore() > s[(j + 1)].getScore()) {
                int tmp = s[(j + 1)].getScore();
                s[(j + 1)].setScore(s[j].getScore());
                s[j].setScore(tmp);
                string tmp1 = s[(j + 1)].getName();
                s[(j + 1)].setName(s[j].getName());
                s[j].setName(tmp1);
            }
        }
    }
}

Your `students` is a pointer to `Student`. Make it an array once you know the `count` using `students = new Student[count];`. — Honza Remeš, Apr 26 '16 at 14:49
There's no need to do `count = new int;` Just declare it as `int count`; — Sean, Apr 26 '16 at 14:49
`count = new int;` -- You or your teacher has gone overboard with the pointer requirements if you write code like this. Also, they didn't teach `delete`? — PaulMcKenzie, Apr 26 '16 at 14:52
Side note: you can assign `Student`s to each other; `Student tmp = s[j+1];` is nicer than mucking around with getters and setters. — molbdnilo, Apr 26 '16 at 14:55
@molbdnilo Or if you just want to swap the two you could use... the `std::swap` function. — Bartek Banachewicz, Apr 26 '16 at 15:16
Don't use `students = new Student[count]`; use `vector students; /* ... */ students.resize(count);`. If your teacher disagrees, ask them to explain why they think beginning programmers should be worrying about manual memory management, and inviting memory leaks by not `delete`ing, when the stdlib makes this unnecessary in almost all situations. The answer is bound to be entertaining and/or horrifying. — underscore_d, Apr 26 '16 at 15:28

Bartek Banachewicz · Answer 1 · 2016-04-26T15:39:17.387

First off, your Student class can be simplified to this:

struct Student {
    double score;
    std::string name;
};

Because the accessors do absolutely nothing. I've also added the std:: prefix because using namespace std is considered a bad practice.

Now, instead of using the pointer to store the students, include vector and use that:

std::cout << "How many students? ";
int count;
std::cin >> count;

std::vector<Student> students(count);

The loading routine can also be simplified given the absence of accesors:

for (auto& student : students) {
    std::cout << "Please enter the students name: ";
    std::cin >> student.name;
    std::cout << "Please enter " << student.name << "'s score: ";
    std::cin >> student.score;
    while (score < 0) {
        std::cout << "ERROR: Score must be a positive number.\n";
        std::cin >> student.score;
    }
}

And actually once you have that, you could just put it in istream& operator>>(istream&, Student&) and reduce it to:

std::copy_n(std::istream_iterator<Student>(std::cin), students.size(), students.begin());

No need now for temporary variables anymore (and even if you want to use them, they should be defined just before the use, so inside of the loop).

The last thing is your sorting routine. First off, there's std::sort that you can use instead if you simply provide a comparator:

std::sort(
    begin(students),
    end(students),
    [](Student const& a, Student const& b) { return b.score < a.score; }
);

If you insist on writing the sorting routine yourself, at least use std::swap.

class pointer unitialized local variable

1 Answers1