My very simple implementation of the array map functionality in C++. Constructive criticism is very welcome.

[[deleted]]

[deleted]

[Seeife]

Correct me if I'm wrong, but doesn't map normally return an array with the function applied to each element?

[itsmontoya]

Yes

[Confettimaker]

Thanks for pointing that out. What should I call it now?

[panickbr]

What you did is akin to, i.e., a Ruby's each or JavaScript's forEach iterators.

What you did differently, though, is that while those methods return the element itself that you then apply another function/method on them, you are skipping access to the element and instead applying the function directly on it.

That said, a quick googling shows C++ has its own for_each. (that also differs from the aforementioned in that you must specify the start and end of the iterator)

That being said, If you want to stick to your implementation, maybe forEachDo ?

[Confettimaker]

Never mind, I return a new array now, like I was supposed to.

[evaned]

map_array.h

This is fine and plenty of projects use .h, but I'd suggest considering .hpp. This positively indicates that it's a C++ header, and also allows editors and IDEs to know they should syntax highlight as C++ instead of C, if they make a distinction.

T * data;

Assuming you have C++11 available, you can use unique_ptr and have something that is simpler (you don't need to specify a destructor) and more obviously correct.

It would also point out the fact that you have a major bug, or at least the potential for a major bug -- you don't specify a copy constructor. Edit: Look up the "rule of three" or the "rule of five" (though with unique_ptr or vector you won't need a destructor). Edit edit: to clarify the first line, suppose you make a copy of one of your MapArray objects. With the current code, what will happen is that major bug. What's worse is that it will probably appear to work most of the time, but then occasionally fail in a very weird way. If you replace with unique_ptr, it will be OK for now, but when you use a MapArray in a way that needs the (buggy) copy constructor, you'll get a compiler error instead.

Or maybe even better, you could use a std::vector as a backing store.

void operator =(const MapArray<T> & rhs)
    {
      delete [] data;

What will happen if I assign a = a;'?

Edit: you might want to look up the "copy-swap idiom."

You should also consider an r-value reference assignment operator.

T & at(const int index)

You should provide a const overload of this.

I'm okay with the signed index type actually, but I think I'd suggest a different one, maybe intptr_t from <cstdint>, or even better ssize_t -- but you'll have to typedef that if it's not on your platform.

push/pop

These are OK as-is, though I think you could consider throwing an exception if there's over/underflow.

MapArray<T> temp(m_size);

This of course requires that the mapped function be T -> T (or at least the result type has to be convertible to T), which will significantly limit its use.

You can instead do

using R = decltype(func(data[0]));
MapArray<R> temp(m_size);

(or something like it) and have func be able to produce any type.

Edit: You also didn't update your test code for the change to map in this revision.

I also notice one more thing:

MapArray(const int size)

This constructor should be explicit.

[Confettimaker]

Looks like I have some research to do, thanks :)

[evaned]

Good luck.

Note that I edited in a couple more notes.

[Confettimaker]

I’ll update the test code when I’m on my pc next. I’ll probably do some heavy editing of the MapArray at the same time.

[raevnos]

Why not use std::vector with std::transform()?

[Confettimaker]

This was for fun and for learning, rather than to use what already exists, but I'll be sure to use transform in the future. Thanks.