Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

No need to rewrite what Yuushi already mentioned Yuushi already mentioned, so, I will add few observations:

You seem to be creating something like boost::static_vector with one exception: your rem (remove) is like swap(at(i), back()); pop_back() - which is faster than simple erase(), but will reorder items.


I suppose that you want your objects destroyed immediatelly, therefore std::array<T,N> would not be an option.


I have only one objection about the code:

template <typename T, unsigned int N>
char* SwapArray<T,N>::memAt( unsigned int i )
{
 auto ct = static_cast<const SwapArray<T,N>*> (this);
 return const_cast<char*> ( ct->memAt(i) );
}
template <typename T, unsigned int N>
const char* SwapArray<T,N>::memAt(unsigned int i) const
{
 ktcAssert( i < N );
 return static_cast<const char*>( m_alignedMem.address() ) + (i * sizeof(T));
}

This const_cast pattern can be seen everywhere. I understand that it could be good not to duplicate big code, but in this case (when it is quite short), copy-pasting two lines and adapting would be better. I would not use const_cast here.


It is getting too long there in comments, so, I will try to give one example against the memcpy:

class example {
 int first;
 int second;
 int *which;
public:
 example(): first(1), second(2) { which = &first; }
 example(const example& src): first(src.first), second(src.second) {
 which = src.which == &src.first ? &first : &second; }
};

Now we have class that will work in std::vector but your memcpy will break it.

No need to rewrite what Yuushi already mentioned, so, I will add few observations:

You seem to be creating something like boost::static_vector with one exception: your rem (remove) is like swap(at(i), back()); pop_back() - which is faster than simple erase(), but will reorder items.


I suppose that you want your objects destroyed immediatelly, therefore std::array<T,N> would not be an option.


I have only one objection about the code:

template <typename T, unsigned int N>
char* SwapArray<T,N>::memAt( unsigned int i )
{
 auto ct = static_cast<const SwapArray<T,N>*> (this);
 return const_cast<char*> ( ct->memAt(i) );
}
template <typename T, unsigned int N>
const char* SwapArray<T,N>::memAt(unsigned int i) const
{
 ktcAssert( i < N );
 return static_cast<const char*>( m_alignedMem.address() ) + (i * sizeof(T));
}

This const_cast pattern can be seen everywhere. I understand that it could be good not to duplicate big code, but in this case (when it is quite short), copy-pasting two lines and adapting would be better. I would not use const_cast here.


It is getting too long there in comments, so, I will try to give one example against the memcpy:

class example {
 int first;
 int second;
 int *which;
public:
 example(): first(1), second(2) { which = &first; }
 example(const example& src): first(src.first), second(src.second) {
 which = src.which == &src.first ? &first : &second; }
};

Now we have class that will work in std::vector but your memcpy will break it.

No need to rewrite what Yuushi already mentioned, so, I will add few observations:

You seem to be creating something like boost::static_vector with one exception: your rem (remove) is like swap(at(i), back()); pop_back() - which is faster than simple erase(), but will reorder items.


I suppose that you want your objects destroyed immediatelly, therefore std::array<T,N> would not be an option.


I have only one objection about the code:

template <typename T, unsigned int N>
char* SwapArray<T,N>::memAt( unsigned int i )
{
 auto ct = static_cast<const SwapArray<T,N>*> (this);
 return const_cast<char*> ( ct->memAt(i) );
}
template <typename T, unsigned int N>
const char* SwapArray<T,N>::memAt(unsigned int i) const
{
 ktcAssert( i < N );
 return static_cast<const char*>( m_alignedMem.address() ) + (i * sizeof(T));
}

This const_cast pattern can be seen everywhere. I understand that it could be good not to duplicate big code, but in this case (when it is quite short), copy-pasting two lines and adapting would be better. I would not use const_cast here.


It is getting too long there in comments, so, I will try to give one example against the memcpy:

class example {
 int first;
 int second;
 int *which;
public:
 example(): first(1), second(2) { which = &first; }
 example(const example& src): first(src.first), second(src.second) {
 which = src.which == &src.first ? &first : &second; }
};

Now we have class that will work in std::vector but your memcpy will break it.

added 504 characters in body
Source Link
user52292
user52292

No need to rewrite what Yuushi already mentioned, so, I will add few observations:

You seem to be creating something like boost::static_vector with one exception: your rem (remove) is like swap(at(i), back()); pop_back() - which is faster than simple erase(), but will reorder items.


I suppose that you want your objects destroyed immediatelly, therefore std::array<T,N> would not be an option.


I have only one objection about the code:

template <typename T, unsigned int N>
char* SwapArray<T,N>::memAt( unsigned int i )
{
 auto ct = static_cast<const SwapArray<T,N>*> (this);
 return const_cast<char*> ( ct->memAt(i) );
}
template <typename T, unsigned int N>
const char* SwapArray<T,N>::memAt(unsigned int i) const
{
 ktcAssert( i < N );
 return static_cast<const char*>( m_alignedMem.address() ) + (i * sizeof(T));
}

This const_cast pattern can be seen everywhere. I understand that it could be good not to duplicate big code, but in this case (when it is quite short), copy-pasting two lines and adapting would be better. I would not use const_cast here.


It is getting too long there in comments, so, I will try to give one example against the memcpy:

class example {
 int first;
 int second;
 int *which;
public:
 example(): first(1), second(2) { which = &first; }
 example(const example& src): first(src.first), second(src.second) {
 which = src.which == &src.first ? &first : &second; }
};

Now we have class that will work in std::vector but your memcpy will break it.

No need to rewrite what Yuushi already mentioned, so, I will add few observations:

You seem to be creating something like boost::static_vector with one exception: your rem (remove) is like swap(at(i), back()); pop_back() - which is faster than simple erase(), but will reorder items.


I suppose that you want your objects destroyed immediatelly, therefore std::array<T,N> would not be an option.


I have only one objection about the code:

template <typename T, unsigned int N>
char* SwapArray<T,N>::memAt( unsigned int i )
{
 auto ct = static_cast<const SwapArray<T,N>*> (this);
 return const_cast<char*> ( ct->memAt(i) );
}
template <typename T, unsigned int N>
const char* SwapArray<T,N>::memAt(unsigned int i) const
{
 ktcAssert( i < N );
 return static_cast<const char*>( m_alignedMem.address() ) + (i * sizeof(T));
}

This const_cast pattern can be seen everywhere. I understand that it could be good not to duplicate big code, but in this case (when it is quite short), copy-pasting two lines and adapting would be better. I would not use const_cast here.

No need to rewrite what Yuushi already mentioned, so, I will add few observations:

You seem to be creating something like boost::static_vector with one exception: your rem (remove) is like swap(at(i), back()); pop_back() - which is faster than simple erase(), but will reorder items.


I suppose that you want your objects destroyed immediatelly, therefore std::array<T,N> would not be an option.


I have only one objection about the code:

template <typename T, unsigned int N>
char* SwapArray<T,N>::memAt( unsigned int i )
{
 auto ct = static_cast<const SwapArray<T,N>*> (this);
 return const_cast<char*> ( ct->memAt(i) );
}
template <typename T, unsigned int N>
const char* SwapArray<T,N>::memAt(unsigned int i) const
{
 ktcAssert( i < N );
 return static_cast<const char*>( m_alignedMem.address() ) + (i * sizeof(T));
}

This const_cast pattern can be seen everywhere. I understand that it could be good not to duplicate big code, but in this case (when it is quite short), copy-pasting two lines and adapting would be better. I would not use const_cast here.


It is getting too long there in comments, so, I will try to give one example against the memcpy:

class example {
 int first;
 int second;
 int *which;
public:
 example(): first(1), second(2) { which = &first; }
 example(const example& src): first(src.first), second(src.second) {
 which = src.which == &src.first ? &first : &second; }
};

Now we have class that will work in std::vector but your memcpy will break it.

Source Link
user52292
user52292

No need to rewrite what Yuushi already mentioned, so, I will add few observations:

You seem to be creating something like boost::static_vector with one exception: your rem (remove) is like swap(at(i), back()); pop_back() - which is faster than simple erase(), but will reorder items.


I suppose that you want your objects destroyed immediatelly, therefore std::array<T,N> would not be an option.


I have only one objection about the code:

template <typename T, unsigned int N>
char* SwapArray<T,N>::memAt( unsigned int i )
{
 auto ct = static_cast<const SwapArray<T,N>*> (this);
 return const_cast<char*> ( ct->memAt(i) );
}
template <typename T, unsigned int N>
const char* SwapArray<T,N>::memAt(unsigned int i) const
{
 ktcAssert( i < N );
 return static_cast<const char*>( m_alignedMem.address() ) + (i * sizeof(T));
}

This const_cast pattern can be seen everywhere. I understand that it could be good not to duplicate big code, but in this case (when it is quite short), copy-pasting two lines and adapting would be better. I would not use const_cast here.

lang-cpp

AltStyle によって変換されたページ (->オリジナル) /