Vector of Card classes for Uno

Question

I'm currently in the process of making Uno for a programming project. One part requires that I make the deck of 108 cards, where Card is its own class with a char color and an std::string value, the values of which I get from a file. I currently am just creating all 108 cards individually and then using push_back to put them in the cards vector. Is there any way to more efficiently make the cards and put them into the vector? Currently it takes up 324 lines

Draw.cpp

#include "Pile.h"
#include "Draw.h"
#include "Card.h"
#include <iostream>
#include <fstream>
#include <string>

Draw::Draw(const std::string FILENAME)
{
    std::ifstream inFile;

    inFile.open(FILENAME);

    if (inFile)
    {
        char newColor;
        std::string newValue;
        inFile >> newColor >> newValue;
        Card card1(newColor, newValue);
        this->cards.push_back(card1);
        inFile >> newColor >> newValue;
        Card card2(newColor, newValue);
        this->cards.push_back(card2);
        ...
        inFile >> newColor >> newValue;
        Card card107(newColor, newValue);
        this->cards.push_back(card107);
        inFile >> newColor >> newValue;
        Card card108(newColor, newValue);
        this->cards.push_back(card108);
    }
    else
    {
    std::cout << "ERROR:" << std::endl;
    std::cout << "The file " << FILENAME << " does not exist." << std::endl;
    std::cout << "The deck cannot be created." << std::endl;
    }
}

@ThomasSablik That was the only answer at the time and the comments on it fixed what was wrong with it so I just accepted it the answer. Since there are other ones now I made one that was more correct the answer — Jokaleth, May 03 '20 at 00:55

score 1 · Answer 1 · answered May 03 '20 at 00:03

1

You can use for loop.

for (int i = 0; i < 108; i++) {
    char newColor;
    std::string newValue;
    inFile >> newColor >> newValue;
    Card card(newColor, newValue);
    this->cards.push_back(card);
}

answered May 03 '20 at 00:03

MikeCAT

73,922
11
45
70

hanabanashiku · Accepted Answer · 2020-05-03T13:24:27.443

As far as reading from a file and instantiating objects line by line goes, the code you’ve written is correct. However if I’m correct in assuming your goal is to make the code readable/more concise, a for loop would go a long way.

For loops are code blocks that will automatically increment a counter variable for you until a certain condition is hit. In this case, it might look something like this:

int i;

for(i = 0; i < 108; i++) {
    char newColor;
    std::string newValue;
    inFile >> newColor >> newValue;
    Card card(newColor, newValue);
    this->cards.push_back(card);
}

Note that this will fail (as will your other implementation) if the input file has any missing values. You can check for this by adding conditionals like this:

if(!(inFile >> newColor >> newValue)) {
    break;
}
else {
   this->cards.push_back(card);
}

IMO always test for a positive instead of a negative. It's easier to read and understand. The else shouldn't ever be the expected correct behavior case. Since you're in a loop you don't need the else at all. You'll get there because you didn't break. — Retired Ninja, May 03 '20 at 00:22

score -2 · Answer 3 · answered May 02 '20 at 23:59

-2

while (inFile)
{
    char newColor;
    std::string newValue;
    inFile >> newColor >> newValue;
    Card card(newColor, newValue);
    this->cards.push_back(card);
}

is all you need

answered May 02 '20 at 23:59

pm100

48,078
23
82
145

1

This code have problem like [c - Why is “while ( !feof (file) )” always wrong? - Stack Overflow](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) – MikeCAT May 03 '20 at 00:02
You should add a check after `inFile >> newColor >> newValue;`, e.g. `if (!inFile) break;` – Thomas Sablik May 03 '20 at 00:03
I'd make the loop based on a successful read to avoid adding a bogus card at the end if the read fails.. `while (inFile >> newColor >> newValue) { ... }` – Retired Ninja May 03 '20 at 00:03
Oh is that really it? I was going to do that but I thought it wouldn't work. Looking at it now I see where my mistaken thinking was. Thanks! – Jokaleth May 03 '20 at 00:13

Vector of Card classes for Uno

3 Answers3