0
// All right? This is really good working code? 
//Need init array with value "false"

bool **Madj;
int NodeCount=4;

bool **Madj = new bool*[NodeCount];
for (int i=0; i<NodeCount; i++){
    Madj[i] = new bool [NodeCount];
    for (int j=0; j<NodeCount; j++){
        Madj[i][j] = false;
    }
}
3
  • 2
    Have you tested it? What is the error and what is your question? Commented Apr 14, 2011 at 19:26
  • 3
    Yes, this will create a 4x4 matrix of booleans. No, it's not particularly good; Madj is declared twice without need and the code is prone to memory leaks. Commented Apr 14, 2011 at 19:26
  • @Emile: it does, but I'm not sure enough it doesn't belong here to vote to close. Code Review is still in beta so we can't refer questions there. Commented Apr 14, 2011 at 19:39

6 Answers 6

4

You could consider using Boost's builtin multi-dimensional array as a less brittle alternative. As noted the code you supplied will work, but has issues.

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

Comments

4

What about:

std::vector<std::vector<bool> >   Madj(4,std:vector<bool>(4, false));

Unfortunately std::vector<bool> is specialized to optimize for size (not speed).
So it can be inefficient (especially if used a lot). So you could use an int array (if you find the bool version is slowing you down).

std::vector<std::vector<int> >   Madj(4,std:vector<int>(4, 0));

Note: int can be used in a boolean context and auto converted (0 => false, any other number is true (though best to use 1).

Comments

3

At least IMO, if you insist on doing this at all, you should normally do it rather differently, something like:

class bool_array { 
     bool *data_;
     size_t width_;

     // no assignment or copying
     bool_array &operator=();
     bool_array(bool_array const &);
public:
     bool_array(size_t x, size_t y) width_(x) {
         data_ = new bool[x*y];
         std::fill_n(data_, x*y, false);
     }

     bool &operator()(size_t x, size_t y) { 
         return data_[y+width_+x];
     }

     ~bool_array() { delete [] data_; }
};

This can be embellished (e.g., using a proxy to enforce constness), but the general idea remains: 1) allocate your bools in a single block, and 2) put them into a class, and 3) overload an operator to support reasonably clean indexing into the data.

You should also consider using std::vector<bool>. Unlike other instantiations of std::vector, it's not a container (as the standard defines that term), which can be confusing -- but what you're creating isn't a container either, so that apparently doesn't matter to you.

2 Comments

What is the potential danger of filling that using a for loop in case of a memory leak? Could you point me to some article about that on the web?
@Hossein: I'm not quite sure what you're asking. Part of the idea of encapsulating it into an object is to ensure that memory doesn't leak.
1
bool **Madj = new bool*[NodeCount];
for (int i=0; i<NodeCount; i++){
    Madj[i] = new bool [NodeCount];
    for (int j=0; j<NodeCount; j++){
        Madj[i][j] = false;
    }
}

If the first call to new succeeds but any of the ones in the loop fails, you have a memory leak since Madj and the subarrays up to the current i are not deleted. Use a vector<vector<bool> >, or a vector<bool> of size NodeCount * NodeCount. With the latter option, you can get to element (i,j) with [i*NodeCount+j].

Comments

0

I think this looks fine!

Depending on the use, you could use std::vector instead of a raw array.

But its true that the first Madj declaration should be "extern" to avoid linking or shadowing errors.

5 Comments

It's true, but they are only potential. If you have a vase on your desk it has potential to crash by dropping on the floor.
(referring to the memory leaks I pointed out; I since deleted the comment): you're defending the kind of sloppy programming that in today's world may cost big bucks or lives.
@larsmans The sloppy part is when you forget to reallocate the memory. Relying on automatic safeguards only moves the problem elsewhere. Memory leaks and crashes are only a tiny fraction of all the bugs around, and you don't have automatic safeguards for wrongly implemented logic. I totally support OP's code. I understand it creates a "risk" of a memory dealloc bug but every code line is a potential source for bugs.
The OP's code is not a potential source for bugs, it contains bugs.
@larsmans Does it contain some other bug than the duplicate declaration of Madj that would have been caught by the compiler-linker tool chain?
0

If you have only bools, consider using bitsets. You can combine that with other containers for multidimensional arrays, e.g vector<bitset>.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.