0
\$\begingroup\$

In a personal project I was working on, I had a vector of std::vector< std::pair<unsigned,char>>'s that represented a character and its position in the alphabet. I found that using/manipulating that vector to be clumsy so I wrote this implementation. Essentially, it just wraps the usual vector operations. I'm hoping for the following things to be considered, as well as the usual areas of review:

  • Are there any other operations that should be implemented?
  • I didn't have a C++20 compiler available at the time of writing this, what needs to be done to "modernize" the code?
  • Are there specific ways to change or improve the code?

And a few notices

  • I know that begin() and end() should return a std::vector::iterator, but I was having difficulties making that compile so I kept it as auto for the time begin. (Yes,I know that is a dirty hack. It's on my to do list.
  • There are other operators that need to be overloaded ( = and == especially), but I wanted to improve the existing code before adding more.

The code is licensed under the BSD Clause 3 license, if you wish to use it/change it.

#include <iostream>
#include <vector>

template< typename Left, typename Right>
class Vecpair
{
  public:

    auto begin(void);
    auto end(void);

    void pushBack(Left,Right);
    void pushBack( std::pair<Left,Right> );
    void popBack(void);
    void clear(void);

    std::size_t getSize(void);
    std::pair<Left,Right> atPos(unsigned);
    std::pair<Left,RIght> front(void);
    std::pair<Left,Right> back(void):

    std:::pair<Left,Right> operator [] (unsigned i){ return( internal.at(i) ); }

  private:

    std::size_t numElem = 0;
    std::vector< std::pair<Left,Right>> internal;
};

//  Returns an iterator to the beginning of 'internal'.
template< typename Left, typename Right>
auto Vecpair<Left,Right>::begin(void)
{
  return( internal.begin() );
}

//  Returns an iterator to the end of 'internal'.
template< typename Left, typename Right>
auto Vecpair<Left,Right>::end(void)
{
  return( internal.end() );
}

// Takes two separate values and pushes them to the back of 'internal' as a pair.
template< typename Left, typename Right>
void Vecpair<Left,Right>::pushBack(Left l, Right r)
{
  internal.push_back( {l,r} );
  numElem++;
}

//  Takes a pair and pushes to the back of 'internal'.
template< typename Left, typename Right>
void Vecpair<Left,Right>::pushBack( std::pair<Left,Right> p )
{
  internal.push_back( p );
  numElem++;
}

//  Pops the last element.
template< typename Left, typename Right>
void Vecpair<Left,Right>::popBack(void);
{
  internal.pop_back();
  numElem--;
}

//  Clears 'internal'.
template< typename Left, typename Right>
void Vecpair<Left,Right>::clear(void)
{
  internal.clear();
  numElem = 0;
}

// Returns the number of elements;
template< typename Left, typename Right>
std::size_t Vecpair<Left,Right>::getSize(void)
{
  return( numElem );
}

//  Returns the pair at the given position.
template< typename Left, typename Right>
std::pair<Left, Right> Vecpair<Left,Right>::atPos(unsigned idx)
{
  return( internal.at(idx) );
}

//  Returns the first pair.
template< typename Left, typename Right>
std::pair<Left,Right> Vecpair<Left,Right>::front(void)
{
  return( internal.front() );
}

//  Returns the last pair.
template<typename Left, Typename Right>
std::pair<Left,Right> Vecpair<Left,Right>::back(void)
{
  return( internal.back() );
}
\$\endgroup\$
  • 4
    \$\begingroup\$ I don't see what benefits this provides over just using the original type. Can you give an example of a usage that yours is better for? \$\endgroup\$ – Leo Adberg May 18 at 2:13
3
\$\begingroup\$

I believe you had to put typename before std::vector<...>::iterator to make it compile. iterator is a dependent name, thus compiler needs a hint (well, in reality it doesn't need it in context of type alias declaration, it is just how syntax is defined before C++20).

Interface

Lets address the elephant in the room first. This class trims down the features of std::vector<std::pair<T, U>> without any visible benefit. Things like std::back_inserter will not work because members are named with different name, comparison operators will not work, there are no type aliases inside the class so users will have to write template metaprogramming for simple things, and so on.

I do not see any benefit of using this instead of std::vector<std::pair<T, U>>.

Implementation

There is no need to have void in the parameter list, empty parameter list means the functions doesn't take any parameters.

numElem looks redundant.

Access functions should be const, possibly noexcept.

Some functions are defined inline, some are not, although they are of the same complexity. This might lead to nasty linker issues.

Putting return value in () might get problematic. When using things like decltype, that will actually alter the result.

Designing interfaces

Wrappers are usually made to streamline one way of usage, possibly without losing any expressive power over original. I believe the main idea was to provide a two-way mapping from two continuous ranges. When designing such wrappers, it is important to understand under which constraints the wrappers are going to work. Let me throw some from top of my head:

  1. The input will be in the form {index-like, index-like}

  2. Two ranges are continuous (from 1 to 26, from 'a' to 'z' given ASCII)

  3. It is possible to normalize the values (e.g. substract a value to make the first one 0), for example for letters it will be substract the value of 'a'

  4. No values outside of the given ranges are ever to be queried

  5. The mapping is one-to-one and onto, e.g. for each value on one side, there is corresponding value on the other side, and it is unique.

If the (1) and/or (2) don't hold. one will need to consider std::map and siblings.

The usage scenarios are as following:

  1. User can somehow read/provide the mapping in the form of a range of std::pair<T, U> or as two separate ranges

  2. User wants to query both ways, e.g. mapping[right] would return an object of Left type, and mapping[left] will return an object of Right type: mapping[3] == 'c' && mapping['c'] == 3.

  3. User might want to know if two mapping are equal, e.g. operator== and operator!= should be provided.

  4. Users might want to define some relationships after the object was created, thus insertion functions like define(const Left& left, const Right& right) are needed. Perfect forwarding might be needed.

  5. Users do not modify the relationships (the pair itself) after they are defined

  6. Users might want to remove a relationship

With those, I believe something like this could fit in:

template <typename Left, typename Right>
class mapping
{
    // internals    
public:
    template <typename InputIterator>
    mapping(InputIterator first, InputIterator last); //extract mappings from iteator range

    // C++20 range version here
    // ditto for version where mappings provided as two ranges

    // rule of 0/3/5 depending on the implementation

    const Right& operator[](const Left& left) const; // no need for non-const version
    const Left& operator[](const Right& right) const;

    // returns false if either side is already in some relationship
    bool define(const Left& left, const Right& right) noexcept; 
    // overload for `std::pair` version

    // throws f either side is already in some relationship
    void try_define(const Left& left, const Right& right);
    // overload for `std::pair` version

    // returns true if the relationship existed and now removed
    bool remove(const Left& left, const Right& right) noexcept(destructor(Left) && destructor(Right)); //pseudocode for noexcept
    // pair version
    // throwing version



    template <typename Left, typename Right>
    friend bool operator==(const mapping<Left, Right>& lhs, const mapping<Left, Right>& rhs);

    template <typename Left, typename Right>
    friend bool operator!=(const mapping<Left, Right>& lhs, const mapping<Left, Right>& rhs);

    template <typename Left, typename Right>
    friend void swap(mapping<Left, Right>& lhs, mapping<Left, Right>& rhs);
};

There are other things one might need, since those are basically sets, operations like

  1. Merge

  2. Difference

  3. Intersection

might be needed for the user.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.