5
\$\begingroup\$

I write this enumerator class out of the necessity. I am working on a project that heavily depends on inheritance, abstract (interface) classes, etc. Therefore I needed a unified container that can store objects of different types (all derived from a single base class).

class posted below is aiming to be a C++ implementation of the C# IEnumerator.

I was aiming to achieve the best possible (runtime/execution) performance from this class, and because of that I deliberately omitted some out_of_range checks (Visual Studio 2015, in debug mode checks that error via assertion, because of that I think I don't need to check it at runtime also).

Unfortunately, the most important methods bool fwd_next(), bool rvs_next() are obligated to perform some additional checks. Which may affect performance.

I am just a junior/hobbyist programmer, therefore I am expecting a high level of criticism because I believe it would help me learn more.

I spent some time implementing this class, but if You are aware of some other implementation in a form of a third party library (boost perhaps ? I checked but I couldn't find any...), please let me know.

In the future I will implement a vector<list<T*>::iterator>> in order to store (and keep track on) iterators returned by list<T*>::push_back like functions. I will do this to be able to access randomly any element in list<T*> m_item_list with O(1) complexity.

I would like to hear an opinion on:

If You have an idea on how to implement better functionality in my class, please let me know.

If you see something wrong, ill, or simply unfit with my code, please also let me know.

Everything else, You are willing to point out.

Question:

Is there a way to allocate a continuous memory in advance (just like std::vector and then just dynamically construct new objects in that memory range?

This is the result:

enumerator.hpp:

#pragma once

#ifndef __ENUMERATOR_HPP__
#define __ENUMERATOR_HPP__
/////////////////////////////////////////////////////////////////////////////////////////////////////////
// ENUMERATOR:
// -- INCLUDE:
/////////////////////////////////////////////////////////////////////////////////////////////////////////

// c++ libs:
#include <list>

///////////





/////////////////////////////////////////////////////////////////////////////////////////////////////////
// ENUMERATOR:
/////////////////////////////////////////////////////////////////////////////////////////////////////////
namespace cpplib {
namespace common {

template <typename T>
class enumerator
{
// typedef: 
private:
    using list_      = std::list<T*>;
    using list_fwd_c = typename list_::const_iterator;
    using list_fwd   = typename list_::iterator;
    using list_rvs_c = typename list_::const_reverse_iterator;
    using list_rvs   = typename list_::reverse_iterator;

// members: 
private:
    list_ m_item_list;
    bool  m_flag_first_item;

    // iterators
    list_fwd m_fwd_itr;
    list_rvs m_rvs_itr;

// helper:
private:

    __forceinline
    void _fwd_erase();

    __forceinline
    void _fwd_erase(list_fwd & fwd_itr);

    __forceinline
    void _rvs_erase();

    __forceinline
    void _rvs_erase(list_rvs & rvs_itr);

// constructors:
public:

    __forceinline
    enumerator();

    template <typename U> __forceinline
    enumerator(const enumerator<U> &) = delete;

    __forceinline
    ~enumerator();

// methods: 
public:

    __forceinline
    void clear();

    __forceinline
    bool fwd_erase_current();

    __forceinline
    bool rvs_erase_current();

    template <typename U = T, typename ... arg_list> __forceinline
    void emplace_back(arg_list ... arg_tail);

// methods: 
public:

    template <typename F> __forceinline
    void fwd_for_each(F & func);

    template <typename F> __forceinline
    void rvs_for_each(F & func);

// methods: 
public:

    __forceinline
    T & fwd_current();

    __forceinline
    T & rvs_current();

    __forceinline
    bool fwd_next();

    __forceinline
    bool rvs_next();

    __forceinline
    void fwd_reset();

    __forceinline
    void rvs_reset();

// methods: 
public:

    __forceinline
    const T & fwd_peak() const;

    __forceinline
    const T & rvs_peak() const;

    __forceinline
    const T & fwd_peak_next() const;

    __forceinline
    const T & rvs_peak_next() const;
};

} // !eval
} // !cpplib
///////////





/////////////////////////////////////////////////////////////////////////////////////////////////////////
// ENUMERATOR:
// -- INCLUDE .inl:
/////////////////////////////////////////////////////////////////////////////////////////////////////////

#include "inl/enumerator.inl"

///////////
#endif // !__ENUMERATOR_HPP__

enumerator.inl:

#pragma once

#ifndef __ENUMERATOR_INL__
#define __ENUMERATOR_INL__
/////////////////////////////////////////////////////////////////////////////////////////////////////////
// ENUMERATOR:
// -- INCLUDE:
/////////////////////////////////////////////////////////////////////////////////////////////////////////

///////////





/////////////////////////////////////////////////////////////////////////////////////////////////////////
// ENUMERATOR:
/////////////////////////////////////////////////////////////////////////////////////////////////////////
namespace cpplib {
namespace common {
namespace detail {
} // !detail

template <typename T> __forceinline
enumerator<T>::enumerator() :
    m_item_list(),
    m_flag_first_item(false)
{};

template <typename T> __forceinline
enumerator<T>::~enumerator()
{
    clear();
}





template <typename T> __forceinline
void enumerator<T>::clear()
{
    for (auto token : m_item_list)
    {
        delete token;
    }

    m_item_list.clear();

    // set flag:
    m_flag_first_item  = false;
}

template <typename T> __forceinline
void enumerator<T>::_fwd_erase()
{
    delete *m_fwd_itr;
    m_fwd_itr = m_item_list.erase(m_fwd_itr);
}

template <typename T> __forceinline
void enumerator<T>::_fwd_erase(list_fwd & fwd_itr)
{
    delete *fwd_itr;
    fwd_itr = m_item_list.erase(fwd_itr);
}

template <typename T> __forceinline
void enumerator<T>::_rvs_erase()
{
    delete *m_rvs_itr;
    m_rvs_itr = m_item_list.erase(m_rvs_itr);
}

template <typename T> __forceinline
void enumerator<T>::_rvs_erase(list_rvs & rvs_itr)
{
    delete *rvs_itr;
    rvs_itr = m_item_list.erase(rvs_itr);
}





template <typename T> __forceinline
bool enumerator<T>::fwd_erase_current()
{
    if (m_flag_first_item)
    {
        _fwd_erase();
        return true;
    }
    else
    {
        return false;
    }
}

template <typename T> __forceinline
bool enumerator<T>::rvs_erase_current()
{
    if (m_flag_first_item)
    {
        _rvs_erase();
        return true;
    }
    else
    {
        return false;
    }
}

template <typename T>
template <typename U, typename ... arg_list> __forceinline
void enumerator<T>::emplace_back(arg_list ... arg_tail)
{
    m_item_list.push_back(new U(arg_tail ...));

    if (! m_flag_first_item)
    {
        m_flag_first_item = true;
        m_fwd_itr = m_item_list.begin();
        m_rvs_itr = m_item_list.rbegin();
    }
};





template <typename T>
template <typename F> __forceinline
void enumerator<T>::fwd_for_each(F & func)
{
    for (auto fwd_itr = m_item_list.begin(); fwd_itr != m_item_list.end(); ++fwd_itr)
    {
        func(**fwd_itr);
    }
}

template <typename T>
template <typename F> __forceinline
void enumerator<T>::rvs_for_each(F & func)
{
    for (auto rvs_itr = m_item_list.rbegin(); rvs_itr != m_item_list.rend(); ++rvs_itr)
    {
        func(**rvs_itr);
    }
}





template <typename T> __forceinline
T & enumerator<T>::fwd_current()
{
    return **m_fwd_itr;
}

template <typename T> __forceinline
T & enumerator<T>::rvs_current()
{
    return **m_rvs_itr;
}

template <typename T> __forceinline
bool enumerator<T>::fwd_next()
{
    if (m_flag_first_item)
    {
        if (++m_fwd_itr != m_item_list.end())
        {
            return true;
        }
        else
        {
            m_fwd_itr = --m_item_list.end();
            return false;
        }
    }
    else
    {
        return false;
    }
}

template <typename T> __forceinline
bool enumerator<T>::rvs_next()
{
    if (m_flag_first_item)
    {
        if (++m_rvs_itr != m_item_list.rend())
        {
            return true;
        }
        else
        {
            m_rvs_itr = --m_item_list.rend();
            return false;
        }
    }
    else
    {
        return false;
    }
}

template <typename T> __forceinline
void enumerator<T>::fwd_reset()
{
    m_fwd_itr = m_item_list.begin();
}

template <typename T> __forceinline
void enumerator<T>::rvs_reset()
{
    m_rvs_itr = m_item_list.rbegin();
}





template <typename T> __forceinline
const T & enumerator<T>::fwd_peak() const
{
    return **m_fwd_itr;
}

template <typename T> __forceinline
const T & enumerator<T>::rvs_peak() const
{
    return **m_rvs_itr;
}

template <typename T> __forceinline
const T & enumerator<T>::fwd_peak_next() const
{
    auto fwd_itr = m_fwd_itr;
    return **(++fwd_itr);
}

template <typename T> __forceinline
const T & enumerator<T>::rvs_peak_next() const
{
    auto rvs_itr = m_rvs_itr;
    return **(++rvs_itr);
}


} // !eval
} // !cpplib
///////////
#endif // !__ENUMERATOR_INL__

main.cpp:

#include <iostream>
#include <string>
#include "common_utils/enumerator.hpp"

int main()
{   
    cpplib::common::enumerator<double> test;
    for (int i = 0; i < 5; i++)
    {
        test.emplace_back(i);
    }

    // do-while becasue 1st element is skipped in a while loop.
    do
    {
        std::cout << test.fwd_current() << std::endl;
    } while (test.fwd_next());
    std::cout << std::endl;

    // for each 'naive' implementation:
    test.fwd_for_each(([](auto item) {std::cout << item << std::endl;}));
    std::cout << std::endl;

    test.rvs_for_each(([](auto item) {std::cout << item << std::endl;}));
    std::cout << std::endl;

    // Exit:
    return 0;
}
\$\endgroup\$
1
  • \$\begingroup\$ __forceinline !!! You think you are better than the compiler. The compiler has a very good algorithm. And it works even if you change your code (which __forceinline does not do (because a human has to re-evaluate if the function is still inlinable)). \$\endgroup\$ Commented Jul 29, 2017 at 18:55

1 Answer 1

6
\$\begingroup\$

enumerator::emplace_back()

Methods like push_back or emplace_back don't belong in an enumerator. Like the name says, the enumerator's job is to enumerate a collection, not to provide all the features of a collection. In C# most collections implement IEnumerable, but that only for the features of enumerating them. If you are provided an IEnumerable you are not provided with any methods of modifying the collection.

Forcing optimizations

I was aiming to achieve the best possible (runtime/execution) performance

Have you profiled your code to detect possible issues with performance? Visual Studio includes a nice and easy to use profiler.

The presence of __forceinline on template code is kind of useless. The compiler can inline it if it deems appropriate even without __forceinline. It's also nonstandard.

Code is difficult to read

There are various aspect of your coding style that make the code quite difficult to read:

  • Having #pragma once but also declaring header guards
  • Using big comment sections that serve no purpose like // ENUMERATOR:
  • Mixing abbreviations with normal words: list_rvs & rvs_itr

Building an enumerator versus sticking to C++ iterators

I wouldn't (re)invent an enumerator for use in production C++ projects. Sticking to normal iterators still has advantages:

  • Standard library algorithms work on iterators
  • Iterators can be copied, allowing more flexibility than a C# IEnumerable which is normally only iterated over once before you start to worry about performance issues like iterating it a second time might trigger a new call to the database
  • boost provides helpers to more easily construct iterators for custom collections
  • boost provides iterator adaptors that allow pipeing iterators similar to what you do with LINQ

If the above still don't suit the needs, I would take a look at the Ranges Library

\$\endgroup\$
3
  • \$\begingroup\$ Thank you very much. I did not profile that code... Because I do not have a big enough sample of data. Even if I had, std::List is a slow container to iterate through, therefore the result would just be meaningless. (like I said I am only a "junior programmer" and most probably I am wrong). \$\endgroup\$
    – PatrykB
    Commented Jul 29, 2017 at 16:39
  • \$\begingroup\$ I understand that enumerator might, and most probably is a bad solution to this issue, but I write it in order to manage a dynamic memory allocation. after all, if I want to work with inheritance, interface(abstract class) I need a common container that will store and manage all those allocations. And on top of that the std::list<double *> is not a solution... \$\endgroup\$
    – PatrykB
    Commented Jul 29, 2017 at 16:48
  • \$\begingroup\$ Yes, my code is difficult to read, and there is nothing more left to say. Thank you, I will work on that. In a matter of boost helpers, iterators and Range Library thank you for pointing me in the right direction. I may be a novice, but I have already realized, that in order to build a professional application, you need to use the professional library, and writing your own solutions is never a good idea. \$\endgroup\$
    – PatrykB
    Commented Jul 29, 2017 at 16:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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