Thread: 2d array pointers code review

Look at your function pparray_delete(). It is deleting a single pointer, multiple times. Undefined behaviour.

It would be better to write your code using a std::vector<std::vector<int> >. If you do that, you wouldn't even need to use operators new and delete at all (except to the extent they are used by vector's allocators, which you shouldn't even need to worry about). You would then be able to eliminate the pparray_create() and pparray_delete functions and greatly simplify the implementation of pparray_multiplication_table() and pparray_print() functions.

The style of your code is essentially idiomatic C, albeit using some C++ language features (eg using operator new instead of malloc(), operator delete instead of free()). That is either good or bad, depending on your perspective. But, the approach is some years behind modern idiomatic C++.

You are still invoking undefined behavior. For every new[], there must be a delete[]. For every new, there must be a delete. You must not mix these types.
What grumpy is referring to is that you are basically doing this the "C way." That is, this is how one would do it in C, and since C++ is backwards compatible with C, it is possible to do it in C++ this way, too, but it is considered bad practices because it's easy to get wrong, and there are better ways (eg std::vector).
If you are merely doing this to get a better feel for pointers, then that is fine. But don't consider using this in "production" code, ie any non-toy project that intend to use for some practical purposes.

Originally Posted by Elysia

You are still invoking undefined behavior. For every new[], there must be a delete[]. For every new, there must be a delete. You must not mix these types.

I'm not sure I understand what I still need to do. Do you mean I need to use "delete[] pparray;" instead of "delete pparray;" on line 26? Could you write a version of pparray_delete() that does it correctly? As I've said a few times now, I really don't have much of an idea of what I'm doing

Originally Posted by Elysia

If you are merely doing this to get a better feel for pointers, then that is fine. But don't consider using this in "production" code, ie any non-toy project that intend to use for some practical purposes.

Hahaha, yeah, production code after less than a week of experience, that would be quite something at this rate I doubt I'd trust myself with anything like that for another year.

How did you allocate myarray? With new[].
How you you delete myarray? With delete.
Undefined behavior.
Get my point?

Basically, change this:

Code:

delete pparray;

to:

Code:

delete[] pparray;

Actually, I was going to let you figure it out for yourself, so that you can grasp exactly what you are doing and the difference between new and new[] (and delete and delete[]).
But I guess this also goes to show how error prone this is, and precisely why we do not recommend it.

Originally Posted by Johannes

I'm a total C++ noob, so forgive me if I have no idea what 'idiomatic C' means (I've been 'programming' C++ for less than a week at this point)

Idiomatic is an english word. It means "natural practice or common practice in the language".

C and C++, despite being related, are usually used in different ways. You are doing things in the way of C, not C++.

The difference is akin to the way native speakers of a natural language (english, french, ....) express themselves differently from non-native speakers or, in your case, in ways that natives speakers of other languages would.

Originally Posted by whiteflags

Don't presume to speak for other people.

There are plenty of other people who make exactly the observation and recommendation that Elysia did. I am among them. More noteworthy people than I who do so include Bjarne Stroustrup, Scott Meyers, and Alex Stepanov.

I dunno, whenever I say something about a particular group, I always get in trouble so I just got away from doing that in general. In this case, the only thing making the observation does is insist that the OP not use what he only just learned.

Maybe so. However, it's a subject of debate whether he should have been taught about pointers in the first place. The thing is, in C++, a beginner who has learned about the standard library can do a lot of things, and do them safely, even with knowing nothing about pointers. However, a beginner who has learned about pointers can do stuff, but the techniques are error prone (eg avoiding molesting a pointer) and will eventually need to learn about the standard library anyway.

No question that, in the long run, someone learning C++ needs to know both about the standard library, and about pointers (as well as dynamic memory allocation). But I'm of the view that it is better to teach safe approaches (eg using the standard library) before teaching unsafe approaches (eg pointers, explicit dynamic memory allocation, and pointer arithmetic). When teaching C++, most things involving pointers can be safely left until later (for example, as an advanced topic) without forcing the student to be unproductive.

Of course, that flies in the traditional approach of teaching C - which amounts to teaching unsafe techniques and forcing beginners to experience their own personal baptism by fire by which they learn to avoid the pitfalls. It also flies in the face of folks who insist on teaching C++ in the same way as C (eg as an extended C). But the traditional teaching approach for C (and often for C++) is akin to a carpentry teacher turning on a circular saw and telling students to use it to cut wood, with no other preparation. There are good reasons beginners to carpentry aren't taught that way. And there are also good reasons to suggest that beginners to C++ shouldn't be taught that way either. [There is also a case to suggest that beginners to C shouldn't be taught that way either, but C doesn't have as many safe and effective alternatives to pointers as does C++].

Originally Posted by whiteflags

Don't presume to speak for other people.

The is the general recommendation of many members of the board, hence I feel that the word "we," is appropriate here.
It does, of course not, mean that I speak for everyone, but that I am implying that many, or most, board members feel the same (otherwise they would denounce that claim, obviously).
That said, if you feel like you don't agree with that, feel free to exclude yourself from the definition of we. I'm sorry if you feel that way, though.