Assignment operator VS Destructor + Placement new, where placement new argument is prvalue?

Question

C++ / C#

Hello everyone

Is it a good practice to use a destructor at address + placement new rather than an assignment operator and a temporary object ?
Example code (here buffer is std::byte):

template<typename ...Args>
    void add(Args &&... args)
    {
        if (size < maxSize)
        {
            new (&buffer[sizeof(T) * head]) T(std::forward<Args>(args)...);

            head = (head + 1) % maxSize;
            ++size;
            return;
        }
        
// Assignment operator
//        (*(reinterpret_cast<T *>(&buffer[sizeof(T) * head]))) = T(std::forward<Args>(args)...);

        // Destructor + Placement new
        int32_t index = sizeof(T) * head;
        (reinterpret_cast<T *>(&buffer[index]))->~T();
        new (&buffer[index]) T(std::forward<Args>(args)...);

        head = (head + 1) % maxSize;
    }

In the variant with the assignment operator, 3 methods will be called:
1. Constructor - first a temporary object T is created
2. (Moving if the operator is present (because T(std::forward(args)...) - prvalue)) Assignment operator - an object is transferred from the 1st step to an already existing object 3.
Destructor - The object created at the 1st step is destroyed

Answer 1

There are many requirements for modern code. In particular, the code must meet certain quality criteria in order to have a chance of being added to the main repository as a solution to the problem.
Among such quality criteria are often: compliance with the general style of the code, compliance with the general style of formatting, the availability of documentation or self-documenting, as well as elementary clarity for the reader.
The reader of modern code shouldn't have to wonder what the poison gas is doing here, why the mops, and why the fan is so big [ ? ].
The reader should understand everything almost immediately, he should have as few questions as possible.
The reader's questions arise when he sees something illogical specifically for a given section of code.
When the reader examines the code for a circular buffer, he keeps in mind the rule that the elements being added will either be added to unallocated memory, or will replace elements already located in memory.
It is better not to break this logic for the reader, otherwise questions will come up. Therefore, the most logical way would be to replace the state of an already placed object with a new one using the move operator.

*reinterpret_cast<T*>( &buffer[ sizeof( T ) * head ] ) = T{ std::forward<Args>( args )... };

Yes, there is a Value Initialization and a move operator. However, optimizing this code will reduce everything to a fairly short and as fast code as possible. So don't worry about it at this stage. It is better to worry about the readability of the code for the reader.
The following code will be just as clear:

auto T vicar{ std::forward<Args>( args )... };
std::swap( *reinterpret_cast<T*>( &buffer[ sizeof( T ) * head ] ), vicar );

It won't raise many questions, except for the usage question std::swap, but only from people with little STL habit.
However, such code may result in a translation error if it Tis non-relocatable. Your central bank must have checks for transferability, copyability, and the possibility of exchanging states.
If Tyou can copy but not move, use the copy operator.
And only if you cannot copy or move T, you should use a destructor and an allocating constructor.
I would recommend wrapping all three actions as function template overloads with SFINAE so that Tonly one specific template overload is included for any.

Answer 2

It seems to me that this issue should be considered from the point of view of several positions:

1. Safety with respect to exceptions . What level of assurance should this method provide?
   
2. Performance. Is it critical to get the most efficient code, or can this be sacrificed at this stage?
   
3. Evaluate the design solution in light of known and proven practices, such as SOLID principles
   

To begin with, we determine whether exceptions are used at all in your project, and if not, then you can go straight to the issue of performance. Otherwise, consider each case:

int32_t index = sizeof(T) * head;
        (reinterpret_cast<T *>(&buffer[index]))->~T();
        new (&buffer[index]) T(std::forward<Args>(args)...);
        head = (head + 1) % maxSize;

this variant does not even provide basic exception guarantees. if the constructor throws an exception, then we have already ended the lifetime of the object by calling the destructor, but size and head will not change; which means we are in an invalid state.

int32_t index = sizeof(T) * head;
        (reinterpret_cast<T *>(&buffer[index]))->~T();
        new (&buffer[index]) T(std::forward<Args>(args)...);
        head = (head + 1) % maxSize;

We can fork noexcept for a constructor using SFINAE or if constexpr and in case the constructor is noexcept; then leave this code. Otherwise, you will have to act depending on the required level of guarantees, but the solutions will be quite cumbersome: in addition to the fork, you will need to catch the exception, throw it further, while restoring the object or decrementing counters, etc. Moreover, you may not provide a strong guarantee at all under certain conditions. As you can see, this has already greatly complicated the solution.
Now to another option:

// Assignment operator
//        (*(reinterpret_cast<T *>(&buffer[sizeof(T) * head]))) = T(std::forward<Args>(args)...);

Here the situation is relatively better, because problems can only be at the level of the assignment operator, which at least shifts part of the responsibility to the author of type T. However, using the solution from Evgeny Shatunov , you can relatively easily get a fairly understandable and "clean" code at the level of strong guarantees. It is also worth looking at the copy and swap idiom, as it is close to this problem.
As a result, if you need strict guarantees, you should give preference to the option with a temporary object.
From a performance point of view, if there is no need for maximum optimization, then more understandable code should be preferred. This point is already well covered in the answer by Evgeny Shatunovso I don't see the point in repeating myself. However, formally, if we discard assumptions about optimizations, then the variant with in-place reconstruction (destructor -> constructor) is more optimal, because guaranteed not to require any additional resource allocation from the temporary object and only two operations + no need for relatively complicated shuffle/copy analysis. In the case of swap, we can still get into copying, depending on the relocation of type T.
And the often forgotten, but extremely important point about design. Single responsibility violated hereprinciple, which may have given rise to this question. Those. your method for adding an element can remove / replace elements, which should raise questions. Moreover, you have decided for the client of your API (even if it is yourself) how to handle the overflow exception. Then, for example, you decide that in one place it is worth putting the old elements into a separate queue in another place to pledge or delete not one at a time, but immediately half of the buffer. All this will require rewriting the add method, but why and why? Try to remove this part of the code, replacing it with throwing an exception or returning the success of the operation (a less competent solution, but this is already from the subjective assessment area) and see how the transparency and ease of writing client code for this method increase. It is also worth looking at a similar design solution instd::vector and its pop_back method . Why doesn't it return the removed element?
Total, if performance is important and working with exceptions is not important; then it is reasonable to choose the option with reconstruction; otherwise exchange with a temporary object. But do not always forget about the analysis of the design solution and whether you are solving the right problem at all.