0

I am working with the following class that has a member to construct a list which can be subclassed with a template.

class dynamic_list
{
  template <typename = void>
  struct node
  {
    std::unique_ptr<node<>> next;

    virtual ~node() = default;
    virtual std::unique_ptr<node<>> clone() const = 0;
  };

  template <typename T>
  struct typed_node : node<>
  {
    T value;

    constexpr typed_node(const typed_node<T>& node)
     : value(node.value)
    {
    }

    template <typename Arg>
    constexpr typed_node(const Arg& arg)
     : value(arg)
    {
    }
    
    std::unique_ptr<node<>> clone() const override
    {
      return std::unique_ptr<typed_node<T>>(new typed_node<T>(*this));
    }
  };

  std::unique_ptr<node<>> head_;

public:
  constexpr dynamic_list() = default;

  template <typename T, typename Arg>
  const T& push(const Arg& arg)
  {
    auto new_node = std::unique_ptr<typed_node<T>>(new typed_node<T>(arg));
    auto& value = new_node->value;
    new_node->next = std::move(head_);
    head_ = std::move(new_node);
    return value;
  }
};

I am trying to write a copy constructor for this class and so far i have the following but it's not working:


  dynamic_list(const dynamic_list& list)
  {
    if (list.head_)
    {
      head_ = list.head_->clone();
      auto temp = head_.get();
      auto list_temp = list.head_.get();
      while (list_temp->next)
      {
        temp->next = list_temp->next->clone();
        temp = temp->next.get();
        list_temp = list_temp->next.get();
      }
    }
  }

Any help would be much appreciated

Spiros Tsalikis
  • 45
  • 5
  • 3
    Handy reading: [Does C++11's decltype make clone unnecessary?](https://stackoverflow.com/questions/10424337/does-c11s-decltype-make-clone-unnecessary) It tells you why what you're doing won't work and strongly implies ["Cloning" as a solution](https://stackoverflow.com/questions/24939570/how-to-clone-as-derived-object-in-c). – user4581301 Mar 23 '22 at 16:39
  • 2
    On a side note: using `unique_ptr` for the `node::next` field is dangerous. When the list is destroyed, you will enter into a recursive loop destroying all of the nodes, and if the list is large then you risk a stack overflow error at runtime with all of those nested destructor calls being pushed on the call stack. An iterative loop to destroy each node is safer than a recursive loop. – Remy Lebeau Mar 23 '22 at 17:07
  • You may want to consider using `std::any`. – Raymond Chen Mar 23 '22 at 17:21
  • @RaymondChen std::any in what way? – Spiros Tsalikis Mar 23 '22 at 17:26
  • @RemyLebeau my list most probably won't be that big but i will consider what you said. – Spiros Tsalikis Mar 23 '22 at 17:27
  • The node can hold a `std::any value;` instead of `T value;`. Then the `std::any` copy constructor does the work for you. – Raymond Chen Mar 23 '22 at 18:17
  • @RaymondChen, while that is absolutely right, it also introduces one more indirection on each node, which may be undesirable. – Fatih BAKIR Mar 23 '22 at 18:43
  • Sometimes C++ is scary. – Joseph Larson Mar 23 '22 at 18:44
  • I can only use c++11 :( – Spiros Tsalikis Mar 23 '22 at 20:54
  • First question is why do you want to make the list copyable in the first place? Is this really necessary? – user207421 Mar 24 '22 at 00:39
  • @SpirosTsalikis, the current version seems to be working, what's the issue? https://godbolt.org/z/xxcfh69E8 – Fatih BAKIR Mar 24 '22 at 00:42
  • **Not working** is not an adequate description of your problem. If we don't know what is the problem, how do you think we would answer? – Phil1970 Mar 24 '22 at 02:06

0 Answers0