Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

##I don't think this is technically legal.

I don't think this is technically legal.

##Style:

Style:

##Members:

Members:

##DRY your code

DRY your code

Did not realize how much I disliked that brace style until now.

##Did not realize how much I disliked that brace style until now. SorrySorry I can't keep going.

##I don't think this is technically legal.

##Style:

##Members:

##DRY your code

##Did not realize how much I disliked that brace style until now. Sorry I can't keep going.

I don't think this is technically legal.

Style:

Members:

DRY your code

Did not realize how much I disliked that brace style until now.

Sorry I can't keep going.

added 3 characters in body
Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341

void assign(size_type count, value_type const &value = value_type()) { if (is_unconstructed()) { allocate(count * 1.5); } else { if (count > capacity()) { reallocate(count * 1.5); } wipe_values(); } this->s_end = std::uninitialized_fill_n( this->ms_begin, count, value); }

void assign(size_type count, value_type const &value = value_type())
 {
 if (is_unconstructed())
 {
 allocate(count * 1.5);
 }
 else
 {
 if (count > capacity())
 {
 reallocate(count * 1.5);
 }
 wipe_values();
 }
 this->s_end = std::uninitialized_fill_n(
 this->ms_begin, count, value);
 }
template<typename InIt>
typename std::enable_if<!std::is_integral<InIt>::value, void>::type assign(InIt first, InIt last)
 {
 difference_type count = (last - first);
 if (is_unconstructed())
 {
 allocate(count * 1.5);
 }
 else
 {
 if (count > capacity())
 {
 reallocate(count * 1.5);
 }
 wipe_values();
 }
 this->s_end = std::uninitialized_copy(
 first, last, this->ms_begin);
 }

void assign(size_type count, value_type const &value = value_type()) { if (is_unconstructed()) { allocate(count * 1.5); } else { if (count > capacity()) { reallocate(count * 1.5); } wipe_values(); } this->s_end = std::uninitialized_fill_n( this->ms_begin, count, value); }

template<typename InIt>
typename std::enable_if<!std::is_integral<InIt>::value, void>::type assign(InIt first, InIt last)
 {
 difference_type count = (last - first);
 if (is_unconstructed())
 {
 allocate(count * 1.5);
 }
 else
 {
 if (count > capacity())
 {
 reallocate(count * 1.5);
 }
 wipe_values();
 }
 this->s_end = std::uninitialized_copy(
 first, last, this->ms_begin);
 }
void assign(size_type count, value_type const &value = value_type())
 {
 if (is_unconstructed())
 {
 allocate(count * 1.5);
 }
 else
 {
 if (count > capacity())
 {
 reallocate(count * 1.5);
 }
 wipe_values();
 }
 this->s_end = std::uninitialized_fill_n(
 this->ms_begin, count, value);
 }
template<typename InIt>
typename std::enable_if<!std::is_integral<InIt>::value, void>::type assign(InIt first, InIt last)
 {
 difference_type count = (last - first);
 if (is_unconstructed())
 {
 allocate(count * 1.5);
 }
 else
 {
 if (count > capacity())
 {
 reallocate(count * 1.5);
 }
 wipe_values();
 }
 this->s_end = std::uninitialized_copy(
 first, last, this->ms_begin);
 }
added 3334 characters in body
Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341

##Style:

Having a hard time reading your code

Your indentation style is making it hard to read the code.

template<typename T,
 typename A>
 class vector;

Hard to see the class part. Align the types if you think it is two long. But put the class back under the template.

template<typename T, typename A>
class vector;
// or
template< typename T, // I would use this style only if the number/size
 typename A> // of templates was going to overwhelm the line.
class vector;

Another example:

vector_base(allocator_type const &al)
: ms_begin(pointer()),
s_end(pointer()),
m_end(pointer()),
alloc(al)
 {
 }

Nothing really lines up so I find it hard to parse.

vector_base(allocator_type const &al)
 : ms_begin(pointer())
 , s_end(pointer())
 , m_end(pointer())
 , alloc(al) 
 {
 }

OK. Putting the braces {} indented is something I personally don't like. But that is an accepted standard style so; though I don't personally like it you should be OK with that as long as you are following the coding conventions of your office.

Be consistent with your indenting.

 if (is_unconstructed())
 {
 allocate(count * 1.5);
 }
 else
 {
 if (count > capacity())
 {
 reallocate(count * 1.5);
 }
 wipe_values(); 
 }

Argg. Poop. I just realized you were consistent. But just emphasis why I hate this style.

##Members:

Having a hard time spotting your class members. You need to make those more abvious (and put them at the top). so I can validate that your constructor actually correctly initializes all members.

##DRY your code

The code in these two function looks nearly identical.
You should try and put common code into a single location. So when you fix a bug you only have to fix a bug in one place (not in multiple places).

void assign(size_type count, value_type const &value = value_type()) { if (is_unconstructed()) { allocate(count * 1.5); } else { if (count > capacity()) { reallocate(count * 1.5); } wipe_values(); } this->s_end = std::uninitialized_fill_n( this->ms_begin, count, value); }

template<typename InIt>
typename std::enable_if<!std::is_integral<InIt>::value, void>::type assign(InIt first, InIt last)
 {
 difference_type count = (last - first);
 if (is_unconstructed())
 {
 allocate(count * 1.5);
 }
 else
 {
 if (count > capacity())
 {
 reallocate(count * 1.5);
 }
 wipe_values();
 }
 this->s_end = std::uninitialized_copy(
 first, last, this->ms_begin);
 }

Not sure I like the fact you have different behavior for a newly constructed verses an old vector if (is_unconstructed()). Seems like the stuff for a new vector only belongs in the constructor.

##Did not realize how much I disliked that brace style until now. Sorry I can't keep going.

##Style:

Having a hard time reading your code

Your indentation style is making it hard to read the code.

template<typename T,
 typename A>
 class vector;

Hard to see the class part. Align the types if you think it is two long. But put the class back under the template.

template<typename T, typename A>
class vector;
// or
template< typename T, // I would use this style only if the number/size
 typename A> // of templates was going to overwhelm the line.
class vector;

Another example:

vector_base(allocator_type const &al)
: ms_begin(pointer()),
s_end(pointer()),
m_end(pointer()),
alloc(al)
 {
 }

Nothing really lines up so I find it hard to parse.

vector_base(allocator_type const &al)
 : ms_begin(pointer())
 , s_end(pointer())
 , m_end(pointer())
 , alloc(al) 
 {
 }

OK. Putting the braces {} indented is something I personally don't like. But that is an accepted standard style so; though I don't personally like it you should be OK with that as long as you are following the coding conventions of your office.

Be consistent with your indenting.

 if (is_unconstructed())
 {
 allocate(count * 1.5);
 }
 else
 {
 if (count > capacity())
 {
 reallocate(count * 1.5);
 }
 wipe_values(); 
 }

Argg. Poop. I just realized you were consistent. But just emphasis why I hate this style.

##Members:

Having a hard time spotting your class members. You need to make those more abvious (and put them at the top). so I can validate that your constructor actually correctly initializes all members.

##DRY your code

The code in these two function looks nearly identical.
You should try and put common code into a single location. So when you fix a bug you only have to fix a bug in one place (not in multiple places).

void assign(size_type count, value_type const &value = value_type()) { if (is_unconstructed()) { allocate(count * 1.5); } else { if (count > capacity()) { reallocate(count * 1.5); } wipe_values(); } this->s_end = std::uninitialized_fill_n( this->ms_begin, count, value); }

template<typename InIt>
typename std::enable_if<!std::is_integral<InIt>::value, void>::type assign(InIt first, InIt last)
 {
 difference_type count = (last - first);
 if (is_unconstructed())
 {
 allocate(count * 1.5);
 }
 else
 {
 if (count > capacity())
 {
 reallocate(count * 1.5);
 }
 wipe_values();
 }
 this->s_end = std::uninitialized_copy(
 first, last, this->ms_begin);
 }

Not sure I like the fact you have different behavior for a newly constructed verses an old vector if (is_unconstructed()). Seems like the stuff for a new vector only belongs in the constructor.

##Did not realize how much I disliked that brace style until now. Sorry I can't keep going.

Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341
Loading
lang-cpp

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