HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Template class to demonstrate array manipulation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
templatearraydemonstratemanipulationclass

Problem

Today I didn't pass the test task for the position C++ junior/middle developer. What i did wrong?
Please help improve myself.

Task:


Description.


Write a template class - an array of any type of data, and tests to
demonstrate the work with this class. Should be implemented all the
methods also acceptable to extend the functionality.

Tests:


The solution should show a class with the following tests:


Working with numbers (int):


1.in loop insert 20 random numbers ranging from 0 to 100.


2.sorting the resulting set of numbers in ascending order.


3.remove each second element.


4.insert 10 random numbers ranging from 0 to 100 at random positions.


5.cleaning container.


After each stage you need to display the contents of the array onto the screen.


Working with objects (std: string):


1.in loop insert 15 randomly chosen words consisting of letters in lowercase.


2.sorting a set of words in ascending order.


3.removing each word comprising any of the letters a, b, c, d, e.


4.inserting 3 new randomly selected words on a random position.


After each stage you need to display the contents of the array onto the screen.

Class interface:

template 
class CArray
{

public:

    CArray();

    CArray( const CArray & _array );

    ~CArray();

    void push_back( const TData & _value );

    void insert( unsigned int _index, const TData & _value );

    void erase( unsigned int _index );

    void clear();

    unsigned int size() const;

    TData & operator[](unsigned int _index );
};


Implementation:

```
#ifndef CARRAY_HPP__
# define CARRAY_HPP__

#include
#include
#include
#include
#include

namespace test
{
namespace util
{

///**
class RandGenerator
{
public:
RandGenerator(
unsigned int _upperbound = RAND_MAX
)
: _mUpperbound(_upperbound)
{
std::srand( static_cast(std::time(0)) );
}

int operator()()
{
return std::rand() %

Solution

You should read this: https://lokiastari.com/posts/Vector-ResourceManagementAllocation

note it is part of a series of 5 articles.

  • They did not ask for an iterator. So I am not sure why there is one.



  • Underscore at the bininning of an identifier is never a good idea.



The rules about its usage are complex at best. Also most people don't know them so avoid using them if you can. Most importantly though is that you violated the rules and used a reserved identifier.

  • You code does not implement the rule of three.



There is no assignment operator. To be honest I would have been happier to see move semantics implemented over iterators.

It's hard to see that you made this private. But it's sill wrong. You should not have a body (to make sure you get a linker error if you accidentally use it) or use C++11 = delete; syntax to remove it.

  • Your layout is very non standard for C++



I have seen this style in other languages like javascript but not in C++

The alignment of you parameters just looks funny.

CArray(
    unsigned int _capacity = CARRAY_DEFAULT_CAPACITY
  ): _mArray(static_cast(::operator new[](_capacity * sizeof(TData)))),
     _mCurPos(0), _mEnd(_capacity)
{}

void init_reserve(
    unsigned int _count
  )
{ /* STuff */ 
}


Looks nicer and easier to read like this:

CArray(std::size_t capacity = 512)
    : mArray(allocateSpace(capacity)
    , mCurPos(0)
    , mEnd(capacity)
{}

void init_reserve(std::size_t count)
{ /* STuff */ 
}


  • Use {} for all sub blocks (especially in an interview) But especially in real life. You don't want to debug that situation where the loop or condition dependent on two statements rather than one.



Example: Apple SSL bug

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    goto fail;
    goto fail;


  • Your implementation of init_reserve() does not provide the strong exception guarantee. If something fails your object is left in an invalid state.



  • Your void insert(unsigned int _index,const TData & _value) is broken.



It uses placement new to add the new item. Even though the memory location it uses has already been constructed (it was in use so it should be good). In this case you should be using copy construction (or move construction).

  • Why is the at() const and operator[] non const. I would expect to see two of each (both const and non cost of each).



  • If you are going to define operator>() for symmetry.



  • The _destruct_objects() should probably delete the objects in reverse order.

Code Snippets

CArray(
    unsigned int _capacity = CARRAY_DEFAULT_CAPACITY
  ): _mArray(static_cast<TData*>(::operator new[](_capacity * sizeof(TData)))),
     _mCurPos(0), _mEnd(_capacity)
{}

void init_reserve(
    unsigned int _count
  )
{ /* STuff */ 
}
CArray(std::size_t capacity = 512)
    : mArray(allocateSpace(capacity)
    , mCurPos(0)
    , mEnd(capacity)
{}

void init_reserve(std::size_t count)
{ /* STuff */ 
}
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    goto fail;
    goto fail;

Context

StackExchange Code Review Q#140656, answer score: 3

Revisions (0)

No revisions yet.