Storing dynamic array in vector

Question

I have to display a histogram of a student's grades. I've stored the grades in a dyn. array, but my goal is to store them in a vector. What's the right way of going about this? Hope this makes sense.

Edit:

My attempt at using a vector

void displayHistogram(int minGrade, vector<int> ptrV) {

cout << endl;
for (int i = 0; i <= minGrade; i++) {
    if (ptrV[i] != 0) {
        cout << "Number of " << i << "'s: " << ptrV[i] << endl;
    }
}

}

void histogram() {

int minGrade = 0, grade;
const int grade_max = 100;
vector<int> ptrV(grade_max, 0);

cout << "Enter the student's grades (-1 to stop entering): \n";
do {
    cin >> grade;
    if (grade > minGrade) {
        minGrade = grade;
    }
    if (grade >= 0) {
        ptrV.push_back(grade);
    }
} while (grade != -1);

displayHistogram(minGrade, ptrV);

}

Does the code work now? If so, have you tried changing int *ptr; to std::vector<int> ptr; and start from there? If so, what issues and/or errors did you encounter? (I need to ask, since this code review questions should go to [StackExchange code review])(codereview.stackexchange.com) — PaulMcKenzie
– PaulMcKenzie, Commented Mar 3, 2019 at 7:36
OT.: do while() is a bit unlucky in your case. Why not while ((cin >> grade) && (grade >= 0))? — Scheff's Cat
– Scheff's Cat, Commented Mar 3, 2019 at 7:36
std::vector can be used like an array and has a lot of additional options. Some of them are: can be initialized with a certain size and default values (sufficient for your case), can be initialized like an array, can grow after construction and so on. There are lots of docs and examples beyond the one given in the link and can be found by web search. — Scheff's Cat
– Scheff's Cat, Commented Mar 3, 2019 at 7:39
If you don't need the feature of growing after initialization, std::array might be another option. It provides a similar interface like std::vector (except for the dynamic resizing). — Scheff's Cat
– Scheff's Cat, Commented Mar 3, 2019 at 7:42
@sethFrias ok, post that attempt also with the usage of vector, so that we have something to work with. — PaulMcKenzie
– PaulMcKenzie, Commented Mar 3, 2019 at 7:48

Aziuth · Accepted Answer · 2019-03-03 09:13:32Z

1

Your basic mistake is that you try to force the vector as if it was a raw array. It does stuff for you, let it. It knows it's size, for instance. You don't need

void displayHistogram(int minGrade, vector<int> ptrV) {

    cout << endl;
    for (int i = 0; i <= minGrade; i++) {

Instead, you can use vector::size:

void displayHistogram(vector<int> ptrV) {

    cout << endl;
    for (size_t i=0; i<ptrV.size(); i++) {

(Even better: void displayHistogram(const vector<int>& ptrV) to signify that ptrV is not changed here and to avoid copying it every time you call the function by using a reference.)

(If you wouldn't use the i as it is the grade and if you have a newer compiler, I'd recommend a for each loop instead. Those are usually the way to go, just happens that you have one of the rarer cases in which it isn't.)

Likewise, you first set the size of your vector and then increase it, which to me means that you do not trust it:

 vector<int> ptrV(grade_max, 0);

At this point, you have a vector with a hundred entries that are all zero. You don't need to resize it later if a hundred entries is all you need. vector::push_back resizes it. But note that setting it to a size of 100 means that [100] is not a valid position, the last one is [99], as we start to count at zero. You'd need to set it to a size of 101 to have both zero and hundred as valid addresses.

I'd change your code to:

const int grade_max = 100;
vector<int> ptrV(grade_max+1, 0); //changed it to +1 here as prtV[100] should be legal

cout << "Enter the student's grades (-1 to stop entering): \n";
while (true)
{
    int grade; // put stuff in the smallest scope possible
    cin >> grade;
    if(grade == -1) break; // doing that here means we don't have to think about it anymore - the do while does it at last, I do it at first, handling all the special cases at the start and then assume I have the regular case.
    if(grade < 0 or grade > grade_max) continue; // continue jumps to the top of the most inner loop. Note that I make sure to catch illegal but possible input.

    ptrV[grade] += 1; // personal preference, I use ++ only to iterate
}

displayHistogram(ptrV);

I rewrote the structure, using a while(true), I think the way I did it is more intuitive, but there will be people who disagree with that and who would also write things like

if(grade == -1)
{
    break;
}

and there are some good arguments for that, mostly a good practice routine, always doing the braces to avoid errors. However, I prefer a one liner to reduce verbosity.

One improvement would also be to tell the user about bad input:

if(grade < 0 or grade > grade_max)
{
    cout << "Input not in valid range. Please choose number within 0 to " << grade_max << endl;
    continue;
}

Now, another big thing to do here would be to leave the procedural part, by the way. Go for a class GradeHistogram which has all those functions as a part of it, being called like

GradeHistogram histogram;
histogram.take_input();
histogram.display();

but that is for when you get your code working.

(My answer became more of a review as found on CodeReview, but I think that this is what you need rather than small fixes. This is something I can only recommend you, by the way, putting your code on CodeReview once it works.)

edited Mar 3, 2019 at 9:13

answered Mar 3, 2019 at 8:55

Aziuth

3,9223 gold badges23 silver badges42 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

sethFrias Over a year ago

Thank you for your review & explaining each component. I've adjusted my code and it is not using the displayHistogram fx. correctly, but I will have to do some further adjustments. Perhaps I just need review what vectors have to offer. Cheers

Aziuth Over a year ago

@sethFrias For the start, it's mostly about understanding their constructor, vector::size, vector::push_back and vector::resize. Next step would be to learn the STL functions that work on such containers, like std::sort and std::find, plus maybe erase and insert. When you feel comfortable with those things, you might want to read into iterators of container classes, but that might be too much for the start. If you are interested about the inner workings of the class, consider vector::reserve and vector::shrink_to_fit, but those are not that easy.

Aziuth Over a year ago

@sethFrias Another thing to do for you is to get used to other container classes, such as std::list, std::array and std::map, and their advantages and disadvantages when compared to vector, including Big O notation (Bachmann-Landau-notation) applied on certain actions on those, like an insertion. In the end, it is about "the right tool for the right job".

PaulMcKenzie · Accepted Answer · 2019-03-03 08:14:54Z

1

but my goal is to store them in a vector.

The issue seems to be that you've already sized the vector to hold grade_max entries. However when filling the vector, you are using push_back. By using push_back, you are adding more entries to the end of the vector, which is not what you want to do.

The solution is either

Change this vector<int> ptrV(grade_max, 0); to this vector<int> ptrV; and leave the calls to push_back alone, or
Keep vector<int> ptrV(grade_max, 0); but instead merely use ptrV[i] = grade;

answered Mar 3, 2019 at 8:14

PaulMcKenzie

35.6k4 gold badges26 silver badges49 bronze badges

2 Comments

sethFrias Over a year ago

I tried this and changed my display loop to i < ptrV.size();, and I receive an output like:

sethFrias Over a year ago

changed my display loop to i < ptrV.size();, and I receive an output like: Number of 0's: 10, Number of 1's: 20,

Jesper Juhl · Accepted Answer · 2019-03-03 08:17:11Z

If what you want to show is a histogram, then the easiest thing to do would be to use a std::map from grade to count of grade.

Something like this:

#include <iostream>
#include <map>
int main() {
    std::cout << "Enter the student's grades (-1 to stop entering): \n";
    std::map<int, int> grades_map;
    int input_grade = -1;
    do {
        cin >> input_grade;
        if (input_grade > -1) {
            grades_map[input_grade]++;
        }
    } while (input_grade != -1);

    // Print histogram
    for (const auto& [grade, count] : grades_map) {
        std::cout << "Students with grade << grade << ": ";
        for (int i = 0; i < count; ++i) {
            std::cout << '*';
        }
        std::cout << '\n';
    }
}

Collectives™ on Stack Overflow

Storing dynamic array in vector

3 Answers 3

3 Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

3 Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related