Return to Revisions

As a learner, I think you have done a great job. Here's some suggestions:

General

• Don't use multiple public: labels. It seems your intent is to split the declarations into groups, but that can be achieved better with // iterator, // element access, etc.

• Some member types are missing: size_type, difference_type, value_type.

• Reverse iterator support is missing.

• Try to avoid nonstandard functions like _aligned_malloc. Use portable features — ::operator new, for example. It would be beneficial for you to wrap the allocation and deallocation into functions, so you can have an easier time transitioning when you add allocator support in the future.

Constructors, assignment operators, and the destructor

• Instead of writing the default constructor, it may be better to use in-class member initializers to ensure that the data members aren't left uninitialized accidentally. And it can (and should) be made noexcept:

   Vector() noexcept = default;
  

• size_t should be std::size_t or (properly defined) size_type. It's not common practice in C++ to make parameters const — at least not in the declaration. So instead of

   explicit Vector(const size_t size);
  

  go with

   explicit Vector(size_type count);
  

  (you may noticed that I used count instead of size to avoid name shadowing.)

• There's the (count, value) constructor and the (iterator, iterator) constructor. Where are they? :) And the std::initializer_list constructor.

• The move constructor and assignment operator should be unconditionally noexcept because they don't actually move elements.

• This is usually phrased as reinterpret_cast:

   _container(static_cast<T*>(_aligned_malloc(sizeof(T)* size, alignof(T))))
  

  and by the way, I like to put nontrivial code (like memory allocation) in the function body to avoid order dependency problems, but that is purely a matter of taste.

• Don't reimplement the library:

   try
   {
   for (size_t i = 0; i < size; i += 1)
   {
   new (_container + i) T();
   }
   }
   catch (...)
   {
   cleanup();
   throw;
   }
  

  can be written as

   std::uninitialized_value_construct_n(_container, size);
  

  which is simple to understand and much less error prone. The try block can be removed because the standard library takes care of exception safety.

• Similarly,

   if constexpr (std::is_trivially_copyable_v<T>)
   {
   memcopy_trivially(_container, other._container, other._size);
   }
   else
   {
   try
   {
   for (_size = 0; _size < other._size; _size += 1)
   {
   emplace_back_internal(std::forward<T>(other._container[_size]));
   }
   }
   catch (...)
   {
   cleanup();
   throw;
   }
   }
  

  can be rewritten as

   std::uninitialized_copy_n(other.begin(), other.end(), _container);
  

  the trivial copy optimization should be handled by any decent implementation, so you don't need to worry about it yourself—:)

• Use the copy and swap idiom. This saves you a lot of boilerplate. The move constructor can be simplified:

   template <typename T>
   Vector<T>::Vector(Vector&& other) noexcept
   :Vector{}
   {
   swap(other);
   }
  

  The copy and move assignment operators can be unified:

   template <typename T>
   auto Vector<T>::operator=(Vector other) noexcept -> Vector&
   {
   swap(other);
   return *this;
   }
  

  (the effect of the noexcept here is that move assignment is noexcept while copy assignment is not. Think of it.)

• std::initializer_list assignment operator :)

• This function

  template void Vector::cleanup() { if constexpr (!std::is_trivially_destructible_v) { std::destroy(begin(), end()); }

   _aligned_free(_container);
  

  }

is a standard facility — it should be named clear, made public, and made noexcept. (Did you just "accidentally" implement a feature?)

Any decent implementation should implement the trivial optimization for std::destroy. Don't do it yourself. If your implementation doesn't, you should complain to them ;) In general, if you are calling a library function, you can be 95% sure that all (relatively) trivial optimizations are implemented.

And you can delegate to erase if you want.

The assign functions

Oops, they are missing.

The member access functions

operator[] should not be made noexcept. noexcept doesn't just mean "no exceptions" — it actually means "this function won't fail".

Also, you probably need std::launder at some point.

Capacity

validate is not a standard function and thus should preferably be private.

The logic of the reserve function can be simplified. I am pretty sure you can avoids the if else if else if else chain by refactoring the code somehow. And

else
{
 // ?
 throw;
}

That's dead code, isn't it? The comment that consists of a single question mark confuses me even more.

Oh, and shrink_to_fit.