3
\$\begingroup\$

I've written a modulus-type program (using std::vector). The user inputs a number, and the program displays a modulus grid pertaining to that number.

This is what it looks like (example program output with 12):

(0)  12 24  36  48  60  72  84  96  108  120
(1)  13  25  37  49  61  73  85  97  109  121
(2)  14  26  38  50  62  74  86  98  110  122
(3)  15  27  39  51  63  75  87  99  111  123
(4)  16  28  40  52  64  76  88  100  112  124
(5)  17  29  41  53  65  77  89  101  113  125
(6)  18  30  42  54  66  78  90  102  114  126
(7)  19  31  43  55  67  79  91  103  115  127
(8)  20  32  44  56  68  80  92  104  116  128
(9)  21  33  45  57  69  81  93  105  117  129
(10)  22  34  46  58  70  82  94  106  118  130
(11)  23  35  47  59  71  83  95  107  119  131
(12)  24  36  48  60  72  84  96  108  120  132

Now, I've decided to make it nicer by using std::map and std::array. I get the same output as before, but with the same alignment issues (fixing those could complicate my display function).

Is this an effective use of these containers (and am I using them correctly)? If not, what other containers could be used instead? I'm using std::array instead of std::vector here because I'm keeping the number of columns (array size) at 10.

#include <iostream>
#include <map>
#include <array>

const unsigned NUM_ARR_ELEMS = 10;

std::map<unsigned, std::array<unsigned, NUM_ARR_ELEMS>> getModGrid(unsigned);
void displayModGrid(const std::map<unsigned, std::array<unsigned, NUM_ARR_ELEMS>>&);

int main()
{
    std::map<unsigned, std::array<unsigned, NUM_ARR_ELEMS>> modGrid;
    unsigned mod;

    std::cout << "\n\n> Mod: ";
    std::cin >> mod;

    std::cout << std::endl << std::endl;
    modGrid = getModGrid(mod);
    displayModGrid(modGrid);

    std::cout << std::endl << std::endl;
    std::cin.ignore();
    std::cin.get();
}

std::map<unsigned, std::array<unsigned, NUM_ARR_ELEMS>> getModGrid(unsigned mod)
{
    std::map<unsigned, std::array<unsigned, NUM_ARR_ELEMS>> modRow;
    std::array<unsigned, NUM_ARR_ELEMS> modValues;

    for (unsigned modIter = 0; modIter <= mod; ++modIter)
    {
        for (unsigned arrIter = 0; arrIter < NUM_ARR_ELEMS; ++arrIter)
            modValues[arrIter] = modIter + (mod * (arrIter+1));

        modRow[modIter] = modValues;
    }

    return modRow;
}

void displayModGrid(const std::map<unsigned, std::array<unsigned, NUM_ARR_ELEMS>> &modGrid)
{
    for (auto rowIter = modGrid.cbegin(); rowIter != modGrid.cend(); ++rowIter)
    {
        std::cout << "  (" << rowIter->first << ")  ";

        for (auto colIter = rowIter->second.cbegin(); colIter != rowIter->second.cend(); ++colIter)
        {
            std::cout << *colIter << "  ";
        }

        std::cout << std::endl;
    }
}
\$\endgroup\$
  • \$\begingroup\$ How about using a range-based for loop? \$\endgroup\$ – Vaughn Cato May 29 '13 at 13:41
  • \$\begingroup\$ My compiler doesn't seem to support it. \$\endgroup\$ – Jamal May 29 '13 at 14:00
  • \$\begingroup\$ How about using a vector of arrays instead of a map? \$\endgroup\$ – Vaughn Cato May 29 '13 at 14:10
  • \$\begingroup\$ That's another option. I went with std::map at first because the left-hand column works well as key values. I'll try this out. \$\endgroup\$ – Jamal May 29 '13 at 14:13
  • \$\begingroup\$ @VaughnCato, I got it working. I even allowed the user to specify a number of columns (excluding the very first one). Should I post that code under the original, in case it may need further analysis? \$\endgroup\$ – Jamal May 29 '13 at 17:37
2
\$\begingroup\$
  • main() can be defined lastly, so that the function prototypes can be removed. This will also allow NUM_ARR_ELEMS to be initialized within main(). The size of either the inner or outer structure can still be accessed via size().

  • For simple input like this, a command line argument is another option:

    int main(int argc, char *argv[])
    {
        unsigned int mod;
    
        // only argument being the file name
        if (argc < 2)
        {
            std::cin << mod;
        }
        // more than the file name, where only the second is used
        else
        {
            mod = std::atoi(argv[1]);
        }
    }
    

    Warning: std::atoi() returns an int, but mod is an unsigned int. As this is a mismatch (and should be reported by the compiler), something else should be changed. Either the return value of std::atoi() should be cast to an unsigned int, or mod should just be changed to an int (and adjusted elsewhere). The latter may be a better option here, and the extra unsigned bit really isn't important. Input validation (to avoid a negative value) can be done as well.

  • This type is quite long to copy in different places:

    std::map<unsigned, std::array<unsigned, NUM_ARR_ELEMS>>
    

    so it can become a typedef:

    typedef std::map<unsigned, std::array<unsigned, NUM_ARR_ELEMS>> Grid;
    
  • Prefer "\n" over std::endl for just a newline. The latter also flushes the buffer.

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

After looking through my code output and thinking about the comments, I've decided that a map of arrays isn't best. I can see how a map can still display it in order (first column numbers as the keys), but that's about it. It does make more sense to use a vector of vectors, after deciding that it's okay to let the user control the number of columns calculated and displayed.

| improve this answer | |
\$\endgroup\$

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.