1
\$\begingroup\$

This function will draw a card in the console

vector<string> draw_card(const Carta& carta){
//The class Carta has two members, carta.value and carta.seme
//both are integers representing the card value or suit
//the card is represented on 9 lines where the second will be the
//card name and suite, while the middle will show a pattern
/*
  |-----------|
  |Asso picche|
  |           |
  |     *     |
  |    * *    |
  |   *****   |
  |  *     *  |
  | *       * |
  |           |
*/

    //card opening and closing line
    string card_delimiter = "|-----------|";

    // vector representing the card
    vector<string> v;

    v.push_back(card_delimiter);

    // construct the second line, so that it will fit different cards names
    // and suites
    string nome_seme = create_semi(carta.seme);
    string nome_valore = create_nome_valore(carta.valore);
    string init_string = "|           |";

    //characters in the init_string will be subsittuted according to the string
    //nome_seme and then after a space nome_valore
    uint valore_offset = 1;
    uint seme_offset = nome_valore.size() + valore_offset + 1;

    for(uint i = 0; i < init_string.size(); ++i){

        if(i >= valore_offset && i < nome_valore.size() + valore_offset){
            init_string[i] = nome_valore[i - valore_offset];
        }

        if(i >= seme_offset && i <= nome_seme.size() + seme_offset - 1){
            init_string[i] = nome_seme[i - seme_offset];
        }
    }

    // update the main vector holding the string
    v.push_back(init_string);

    // initialize the pattern for the center drawing
    vector<uint16_t> middle_matrix;
    middle_matrix.push_back(0x0000); //0      b0000000000000000
    middle_matrix.push_back(0x0400); //1024   b0000010000000000
    middle_matrix.push_back(0x0A00); //2560   b0000101000000000
    middle_matrix.push_back(0x1F00); //7936   b0001111100000000
    middle_matrix.push_back(0x2080); //8320   b0010000010000000
    middle_matrix.push_back(0x4040); //16448  b0100000001000000
    middle_matrix.push_back(0x0000); //0      b0000000000000000

    // scroll through the vector
    for(uint i = 0; i < middle_matrix.size(); ++i){
        string line = "|";
        // bitshift the mask to find if the character will be a space or a *
        for(uint16_t mask = 0x8000; mask != 0x0010; mask >>= 1){
            if(middle_matrix[i] & mask){ line += "*"; }
            else{ line += " "; }
        }
        line += "|";
        v.push_back(line);
    }

    // close the card
    v.push_back(card_delimiter);

    //print the card
    for(unsigned int i = 0; i < v.size(); ++i){
        cout << v[i] << endl;
    }

    return v;
}

I'm posting this for a couple of questions, first of all I'm pretty new to C++ and I would like to know if you would write the whole procedure differently, maybe using replace functions or functions to map the bit pattern.

Second this version will print only one pattern, what would you suggest to do for the other 13? What I did now was to take a text file, draw the pattern, realize it could be expressed in binary and now I'm here with this integers.

How would you go to maintain clarity? Shall I read in the pattern from a file? Declare a static array that contains the numbered patterns? Where shall I place the function that reads the values? or maybe create a class that reads from a "pattern" file and stores them in some member variable, and call it afterwards?

\$\endgroup\$
  • \$\begingroup\$ You'll receive better reviews if you show a complete example. For example, I recommend that you edit to show the necessary #include lines, and a main() that shows how to call your function. As it is, we can't even compile the posted code, because you haven't provided the definition of the Carta type! \$\endgroup\$ – Toby Speight Oct 13 '17 at 9:19
1
\$\begingroup\$

Would you write the whole procedure differently?

Absolutely. In it's current form, it does far too much stuff and (at least in my opinion) is not really flexible enough. Every time you try to add something (e.g. other card patterns, loading patterns from a file, change the card border, ...) the function will just get more and more bloated.

Actually, I'd implement it in a class CardPrinter. This way, you can transfer state (e.g. card patterns) between calls in a nicely, encapsulated way. Also, I'd split the functionality into multiple member functions, so it's easier to reason about them.

(paraphrasing:) How would you handle multiple patterns?

For patterns as simple as these, I'd probably not bother to encode them in any special format. Just an array of lines is fine (unless you're programming under heavy memory constraints).

These patterns can be set statically, loaded from somewhere or even generated dynamically. Choose what fits your circumstances best! E.g. loading from a file could allow for different patterns, allowing for customization.

Example:

struct CardPattern {
    std::vector<std::string> lines;
};

class CardPrinter {
    std::vector<CardPattern> patterns;
    std::string delimiter_left;
    std::string delimiter_right;
    std::string delimiter_top;
    std::string delimiter_bottom;
    size_t internal_width;

public:
    CardPrinter(const std::vector<CardPattern>& pats, size_t w = 13, std::string del_left = "|", std::string del_right = "|", std::string del_top = "-", std::string del_bottom = "-")
            : patterns(pats), delimiter_left(del_left), delimiter_right(del_right), delimiter_top(del_top), delimiter_bottom(del_bottom), width(w - del_left.length() - del_right.length()) {
    }

    void print(const Carta& card, std::ostream& stream = std::cout) {
        stream << get_card_visual(card);
    }
private:
    std::string get_card_title(const Carta& card) {
        auto nome_semi = create_semi(card.seme);
        auto nome_valore = create_nome_valore(card.valore);

        return nome_valore + " " + nome_semi;
    }

    const CardPattern& get_card_pattern(const Carta& card) {
        // assuming pattern exists for that value
        return patterns[card.valore];
    }

    std::string format_line(const std::string& line) {
        // just cut off parts that are too long
        // if too short, fill rest with spaces (could be changed, e.g. center text in that case)
        auto visible_part = line.substr(0, internal_width);
        auto filler = std::string(internal_width - visible_part.length(), ' ');

        return delimiter_left + visible_part + filler + delimiter_right + "\n";
    }

    std::string format_card_pattern(const CardPattern& pattern) {
        auto formatted = std::string("");

        for(auto& line : pattern.lines) {
            formatted += format_line(line);
        }

        return formatted;
    }

    std::string format_top() {
        auto top = std::string();
        while(top.length() < internal_width) {
            top += delimiter_top;
        }

        return format_line(top);
    }

    std::string format_bottom() {
        auto bottom = std::string();
        while(bottom.length() < internal_width) {
            bottom += delimiter_bottom;
        }

        return format_line(bottom);
    }

    std::string get_card_visual(const Carta& card) {
        return format_top() + format_line(get_card_title(card)) + format_card_pattern(get_card_pattern(card)) + format_bottom();
    }
};

// Usage example
auto patterns = /* up to you, e.g. */ load_patterns();
auto printer1 = CardPrinter(patterns);
auto printer2 = CardPrinter(patterns, 15, "::", "::", "=", "=");

auto cards = /* get some cards */;
for(auto& card : cards) {
    printer1.print(card);
    printer2.print(card);
}
\$\endgroup\$
  • \$\begingroup\$ Thank you for the kind answer, I have a couple of questions. First why you used a struct when actually a typedef would be enough? or am I not seeing something? Second I thought also to actually use a class to load the pattern, but store them in a text file seemed, not hacky enough. Moreover your extension I think goes beyond what I had in mind, but thanks for the ideas. (btw the cards line are in a vector, so I can concatenate each line to a possible next card) \$\endgroup\$ – Pella86 Oct 13 '17 at 8:41
  • 1
    \$\begingroup\$ @Pella86: Yes, a typedef would have been enough in this case (unless you want to add behavior to patterns) (it was more of an example "this is what you could expand upon", not "you have to do it this way"). With regards to "not hacky enough": This is just an internal representation. While I wouldn't overcomplicate things with a weird compression scheme (e.g. to save them on disc), you are free to do so (but with text files, you could use prettier ASCII art). Just remember: It's you who will have to be able to understand your code if something needs to be changed later on. \$\endgroup\$ – hoffmale Oct 13 '17 at 15:15
  • 1
    \$\begingroup\$ @Pella86: Also, if you want to print multiple cards horizontally, it shouldn't be too hard to adapt the CardPrinter class for that ;) \$\endgroup\$ – hoffmale Oct 13 '17 at 15:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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