Non-sequential template class container C++

I'm pretty new to templates, and admittedly I'm not particularly good at C++, but I've written a little container that does not move allocated objects once they are created, so you can use weak pointers to access elements, and be aware of their existence. I'm sure that much of it can\should be improved, but I lack the experience to know what exactly. Thoughts and critique are welcome.

```
#include
#include
#include

class NSC_OOR_ERR : public std::exception
{
virtual const char* what() const throw()
{
return "NSContainer access out of range";
}
};

class NSC_BA_ERR : public std::exception
{
virtual const char* what() const throw()
{
return "NSContainer out of memory";
}
};

template
class CNSContainer
{
NSC_OOR_ERR NSC_OOR_m;
NSC_BA_ERR NSC_BA_m;
std::vector> pointers_m;
public:
size_t size()
{
return pointers_m.size();
}

template
std::weak_ptr push(Args&&... args)
{
try {
std::shared_ptr ptr(new A_Type(std::forward(args)...));
}
catch (std::bad_alloc& ba) {
throw NSC_BA_m;
}
pointers_m.emplace_back(ptr);
return ptr;
}

void remove(const size_t& key)
{
pointers_m.erase(pointers_m.begin() + key);
}

std::weak_ptr operator[](const size_t& key)
{
try {
return pointers_m[key];
}
catch (const std::out_of_range& oor) {
throw NSC_OOR_m;
}
}
};

int Test()
{
CNSContainer container;

//push into container
container.push(1);
container.push(2);
container.push(3);
container.push(4);

//get size
size_t container_size = container.size();

//iterate
for (size_t i = 0; i wptr = container[i];

if (!wptr.expired())
{
auto sptr = wptr.lock();
int value = *sptr;
std::cout wptr = container[0];

/the pointer now points to nothing/
cont

Thanks for a well-presented question! I'll dive straight in:

Your exceptions are a bit weird. Leaving aside the impenetrable (to me) naming, I think that adding your own exception types derived directly from std::exception just to add your own what() is probably a poor investment of time. If you go this way, I certainly recommend that you subclass the provided std::bad_alloc and std::out_of_range, so that existing code that catches those also catches your equivalents.

Having member variables of exception type in your container instances is decidedly unconventional. We generally create exceptions at the point where they are thrown. This may be important in a debugging environment, if exceptions there are able to carry context information (as happens in Python and Java, for example). In production code, the exception construction will be very low overhead compared with actually raising and catching the exception, so there's little to be gained in speed, and something to be lost by adding unnecessary members to every instance.

A quick skim through the rest of the code draws me to this line:

std::shared_ptr ptr(new A_Type(std::forward(args)...));

Bare new in modern C++ always triggers my interest - here, it is better to use std::make_shared(). This will give improved memory locality to the object and its smart-pointer control block.

This gives my version of your code as follows:

#include 
#include 

template
class CNSContainer
{
    std::vector> pointers_m;
public:
    size_t size()
    {
        return pointers_m.size();
    }

    template 
    std::weak_ptr push(Args&&... args)
    {
        // may throw std::bad_alloc - if so, pointers_m is unchanged
        auto ptr = std::make_shared(std::forward(args)...);
        pointers_m.emplace_back(ptr);
        return ptr;
    }

    void remove(const size_t& key)
    {
        pointers_m.erase(pointers_m.begin() + key);
    }

    // Will throw std::out_of_range if key is invalid
    std::weak_ptr operator[](const size_t& key)
    {
        return pointers_m[key];
    }
};

At this point, there's almost nothing here that's not in the underlying container, and much that's missing (reserve(), iterators, member types, and more). What you end up with is a convenience function for creating a smart-pointer member in a collection:

#include 

template
std::weak_ptr push(Container& c, Args&&... args)
{
    // may throw std::bad_alloc - if so, c is unchanged
    auto p = std::make_shared(std::forward(args)...);
    c.emplace_back(p);
    return p;
}

You might be able to further generalize that to not depend on emplace_back as the insertion method and/or use Concepts to constrain the function and give more informative errors if it's misused.

I'm sorry if the above reduces what you've written to something near-trivial, but I hope you've learnt a lot whilst you were doing it (and that you can apply that to your future projects). Certainly, you seem to have your head around forwarding references, which is more than many can say!

StackExchange Code Review Q#154890, answer score: 4