Skip to main content
We’ve updated our Terms of Service. A new AI Addendum clarifies how Stack Overflow utilizes AI interactions.
added 531 characters in body
Source Link
G. Sliepen
  • 69.5k
  • 3
  • 75
  • 180

This is not a bad attempt at making a vector-like container, except it is lacking support for const and makes unnecessary copies (see below for a more detailed explaination). The variable names are quite well chosen. It's also good to see Doxygen documentation.

Have a look at the API of std::vector and follow it isas closely as possible, not just the syntax but also the semantics. For example, operator[] does not do bounds checking and will not throw, but there is an at() that does do this. While perhaps not mentioned in the requirements for your interview, it would also be nice to add front(), begin() and end(), emplace_back() and so on, so that your class will be a drop-in replacement for a regular std::vector.

Keep it professional

The code and comments look fine except for the "tasty" exceptions. Avoid making jokes in the code; it doesn't look professional, and it might actually be confusing for non-native speakers that might also need to work on the same code.

Have a look at the API of std::vector and follow it is closely as possible, not just the syntax but also the semantics. For example, operator[] does not do bounds checking and will not throw, but there is an at() that does do this. While perhaps not mentioned in the requirements for your interview, it would also be nice to add front(), begin() and end(), emplace_back() and so on, so that your class will be a drop-in replacement for a regular std::vector.

This is not a bad attempt at making a vector-like container, except it is lacking support for const and makes unnecessary copies (see below for a more detailed explaination). The variable names are quite well chosen. It's also good to see Doxygen documentation.

Have a look at the API of std::vector and follow it as closely as possible, not just the syntax but also the semantics. For example, operator[] does not do bounds checking and will not throw, but there is an at() that does do this. While perhaps not mentioned in the requirements for your interview, it would also be nice to add front(), begin() and end(), emplace_back() and so on, so that your class will be a drop-in replacement for a regular std::vector.

Keep it professional

The code and comments look fine except for the "tasty" exceptions. Avoid making jokes in the code; it doesn't look professional, and it might actually be confusing for non-native speakers that might also need to work on the same code.

Source Link
G. Sliepen
  • 69.5k
  • 3
  • 75
  • 180

Use std::size_t for the size of the storage

The template parameter BUFFER_SIZE should have type std::size_t. This will then also consistent with the type of used_elements. std::size_t is almost always the right type for sizes, counts and indices.

Omit void from empty parameter lists

Functions that don't take parameters should be declared as foo(), not foo(void). The latter is necessary for C, but not for C++.

Use = default to generate a default constructor

If you want the compiler to create a default constructor that takes no arguments, use = default to declare it:

Stable_vector() = default;

The same would go for other constructors and for destructors. By explicitly defining a constructor with an empty body, the compiler will treat it as a user-defined constructor, which sometimes might have some subtle consequences, although it doesn't matter for your class.

Use a consistent code style

I see that sometimes your use of spaces is not consistent, which makes the code look a bit messy. You also sometimes have two statements on the same line; try to limit it to one statement per line. Use a consistent code style (which one you use exactly is of less importance). Instead of fixing everything yourself manually, either check if your code editor has a function to reformat the code, or use an external tool such as Artistic Style or ClangFormat.

Remove debug statements from your code

Debug statements are helpful during initial development and for finding bugs, but it makes the code harder to read. I recommend you remove them, and only temporarily add them back as necessary.

Avoid unnecessary copies

Your push_back() function takes data by value, causing a copy to be made. It then makes another copy when storing it in the internal_buffer. Have a look at std::vector::push_back(): you'll note it has two variants, one that takes a const reference, and the other which takes a forwarding reference. The first one just avoids the first copy, the second one also allows you to std::move() data into internal_buffer, which might avoid part of the second copy, if T has a move assignment operator.

The same goes for insert() of course.

Don't let pop_back() return an element

You'll notice that std::vector::pop_back() is a function that returns void. Instead, you're supposed to use back() to get a reference to the back element, allowing you to read it without making a copy, and then you can remove it with pop_back() afterwards.

Add const versions of operator[] and back()

Since your operator[] and back() are not const functions, they can't be used with a const Stable_vector. You have to add const versions of them (that return const T& of course) to solve this issue.

Mark functions that don't throw exceptions noexcept

If you mark functions that will never throw exceptions as noexcept, this will help the compiler generate more optimal code.

Copy the API of STL containers as much as possible

Have a look at the API of std::vector and follow it is closely as possible, not just the syntax but also the semantics. For example, operator[] does not do bounds checking and will not throw, but there is an at() that does do this. While perhaps not mentioned in the requirements for your interview, it would also be nice to add front(), begin() and end(), emplace_back() and so on, so that your class will be a drop-in replacement for a regular std::vector.