1

I'm new with using classes and I encountered a problem while delcaring an array into a class. I want to initialize a char array for text limited to 50 characters and then replace the text with a function.

#ifndef MAP_H
#define MAP_H
#include "Sprite.h"
#include <SFML/Graphics.hpp> 
#include <iostream>

class Map : public sprite
{
private:
    char mapname[50];
    int columnnumber;
    int linenumber;
    char casestatematricia[];

public:
    void setmapname(char newmapname[50]);
    void battlespace(int column, int line);
    void setcasevalue(int col, int line, char value);
    void printcasematricia();

};


#endif

By the way I could initialize my 2d array like that

char casestatematricia[][];

I want later to make this 2d array dynamic where I enter a column number and a line number like that

casestatematricia[linenumber][columnnumber]

to create a battlefield.

this is the cpp code so that you have an idea of what I want to do.

#include "Map.h"
#include <SFML/Graphics.hpp> 
#include <iostream>

using namespace sf;

void Map::setmapname(char newmapname[50])
{
    this->mapname = newmapname;
}
void Map::battlespace(int column, int line)
{

}
void Map::setcasevalue(int col, int line, char value)
{

}
void Map::printcasematricia()
{

}

thank you in advance.

H. Guijt
  • 3,325
  • 11
  • 16
  • 5
    take a look at `std::string` to replace `char` array which contain string. Also, take a look at `std::vector`, `std::list`, `std::map`, ... (ie standard container) instead of using raw container. – Garf365 Aug 03 '16 at 10:27
  • @Garf365 I don't use char[50] for no reason this data has to be serialize later so it has to have a fixed weight. – Pyrrha DaSmash4Player Aug 03 '16 at 10:31
  • 1
    I also have data which I have to serialize and their is no porblem with `std::string`. Because you have some setter, just add condition on size – Garf365 Aug 03 '16 at 10:33
  • 1
    @PyrrhaDaSmash4Player what do you mean with "fixed weight" ? A `std::vector`s data is stored contiguous, ie you can get a c-style array on the fly from it whenever needed. – 463035818_is_not_an_ai Aug 03 '16 at 10:33
  • 4
    If you are gearing up to just write the whole class contents to a file as if it were an array of bytes, that's not generally a good idea. It is supremely non-portable, and it will bite you as soon as you try to use rtti, or virtual functions, or any other non-POD thing. @Garf365 is giving you excellent advise; you should listen to him. – H. Guijt Aug 03 '16 at 10:39

3 Answers3

4

Consider following common practice on this one. Most (e.g. numerical) libraries don't use 2D arrays inside classes. They use dynamically allocated 1D arrays and overload the () or [] operator to access the right elements in a 2D-like fashion. So on the outside you never can tell that you're actually dealing with consecutive storage, it looks like a 2D array. In this way arrays are easier to resize, more efficient to store, transpose and reshape.

Jacques de Hooge
  • 6,750
  • 2
  • 28
  • 45
2

Just a proposition for your problem:

class Map : public sprite
{
private:
    std::string mapname;
    int columnnumber;
    int linenumber;
    std::vector<char> casestatematricia;

    static constexpr std::size_t maxRow = 50;
    static constexpr std::size_t maxCol = 50; 

public:
    Map():
        casestatematricia(maxRow * maxCol, 0)
    {}
    void setmapname(std::string newmapname)
    {
        if (newmapname.size() > 50)
        {
            // Manage error if you really need no more 50 characters..
            // Or just troncate when you serialize!
        }
        mapname = newmapname;
    }

    void battlespace(int col, int row);
    void setcasevalue(int col, int row, char value)
    {
        // check that col and line are between 0 and max{Row|Column} - 1
        casestatematricia[row * maxRow + col] = value;
    }

    void printcasematricia()
    {
        for (std::size_t row = 0; row < maxRow; ++row)
        {
            for (std::size_t col = 0; col < maxCol; ++col)
            {
                char currentCell = casestatematricia[row * maxRow + col];
            }
        }
    }
};

For access to 1D array like a 2D array, take a look at Access a 1D array as a 2D array in C++.

When you think about serialization, I guess you want to save it to a file. Just a advice: don't store raw memory to a file just to "save" time when your relaunch your soft. You just have a non portable solution! And seriously, with power of your computer, you don't have to be worry about time to load from file!

I propose you to add 2 methods in your class to save Map into file

void dump(std::ostream &os)
{
    os << mapname << "\n";
    std::size_t currentRow = 0;
    for(auto c: casestatematricia)
    {
        os << static_cast<int>(c) << " ";
        ++currentRow;

        if (currentRow >= maxRow)
        {
            currentRow = 0;
            os << "\n";
        }
    }
}

void load(std::istream &is)
{
    std::string line;

    std::getline(is, line);
    mapname = line;

    std::size_t current_cell = 0;
    while(std::getline(is, line))
    {
        std::istringstream is(line);
        while(!is.eof())
        {
            char c;

            is >> c;
            casestatematricia[current_cell] = c;

            ++current_cell;
        }
    }
}

This solution is only given for example. They doesn't manage error and I have choose to store it in ASCII in file. You can change to store in binary, but, don't use direct write of raw memory. You can take a look at C - serialization techniques (just have to translate to C++). But please, don't use memcpy or similar technique to serialize

Community
  • 1
  • 1
Garf365
  • 3,619
  • 5
  • 29
  • 41
-1

I hope I get this right. You have two questions. You want know how to assign the value of char mapname[50]; via void setmapname(char newmapname[50]);. And you want to know how to create a dynamic size 2D array.

I hope you are comfortable with pointers because in both cases, you need it.

For the first question, I would like to first correct your understanding of void setmapname(char newmapname[50]);. C++ functions do not take in array. It take in the pointer to the array. So it is as good as writing void setmapname(char *newmapname);. For better understanding, go to Passing Arrays to Function in C++

With that, I am going to change the function to read in the length of the new map name. And to assign mapname, just use a loop to copy each of the char.

void setmapname(char *newmapname, int length) {
    // ensure that the string passing in is not 
    // more that what mapname can hold.
    length = length < 50 ? length : 50;

    // loop each value and assign one by one. 
    for(int i = 0; i < length; ++i) {
        mapname[i] = newmapname[i];
    }
}

For the second question, you can use vector like what was proposed by Garf365 need to use but I prefer to just use pointer and I will use 1D array to represent 2d battlefield. (You can read the link Garf365 provide).

// Declare like this
char *casestatematricia; // remember to initialize this to 0.

// Create the battlefield
void Map::battlespace(int column, int line) {

    columnnumber = column;
    linenumber = line;

    // Clear the previous battlefield.
    clearspace();

    // Creating the battlefield
    casestatematricia = new char[column * line];

    // initialise casestatematricia...
}

// Call this after you done using the battlefield
void Map::clearspace() {
    if (!casestatematricia) return;

    delete [] casestatematricia;
    casestatematricia = 0;
}

Just remember to call clearspace() when you are no longer using it.

Just for your benefit, this is how you create a dynamic size 2D array

// Declare like this
char **casestatematricia; // remember to initialize this to 0.

// Create the battlefield
void Map::battlespace(int column, int line) {

    columnnumber = column;
    linenumber = line;

    // Clear the previous battlefield.
    clearspace();

    // Creating the battlefield
    casestatematricia = new char*[column];
    for (int i = 0; i < column; ++i) {
        casestatematricia[i] = new char[line];
    }

    // initialise casestatematricia...
}

// Call this after you done using the battlefield
void Map::clearspace() {
    if (!casestatematricia) return;

    for(int i = 0; i < columnnumber; ++i) {
        delete [] casestatematricia[i];
    }

    delete [][] casestatematricia;
    casestatematricia = 0;
} 

Hope this help.

PS: If you need to serialize the string, you can to use pascal string format so that you can support string with variable length. e.g. "11hello world", or "3foo".

Community
  • 1
  • 1
TrBBoI
  • 3
  • 1
  • 1
  • Some criticisms (I hope positive): (1) for `setmapname`, don't use a self maid loop, you have some standard functions to copy array into another one (use instead `std::copy`, `std::memcpy` or `std::strncpy`) => always use standard functions instead of home maid one ;) ; (2) avoid use of raw pointer which has some trouble like ownership and risk of memory leaks, use smart pointer (since c++11, or with boost); (3) Use container instead of managing memory yourself => in general, prefer using standard developed and reviewed by many and many people instead of code write by only one person. – Garf365 Aug 03 '16 at 14:55
  • for example, `std::vector` do what you manually do => allocate contiguous memory, deallocate it, manage access (if you use `at` method of `vector` you will get exception if you give an out of range value, more safe than undefined behavior), and have some more stuff like increase memory (realloc), ... But it does it efficiently, a lot of people had worked hard together on to achieve this – Garf365 Aug 03 '16 at 14:59
  • @Garf365, I agree with you. Making the code maintainable is the way to go and us programmers. However, I am under the impression that the OP is still learning c++, a learner. So the way I structured my answer is to hope that you get the basic understanding right and know how to do it without library first. After knowing how it is done. Then by all means use whatever resources out, stl or boost it doesn't matter. – TrBBoI Aug 04 '16 at 16:17