Issue 598: Refactoring List class

[veronikaro]

Solves #598.

[github-actions[bot]]

:robot: Upon creation, pull request description does not have a link to an issue. If there is a related issue, please add it to the description using any of the supported formats.

[veronikaro]

@gavv I would appreciate your feedback to this PR. Thanks!

[gavv]

Hi, thanks for PR!

A few comments:

1. [x] Please fix CI. There is a crash in tests reported by several jobs. (You can reproduce it locally by using --sanitizers option).

2. [x] Some ListImpl methods use ListNodeData, and some use ListNode. For consistency, I suggest to use ListNodeData everywhere.

3. [x] I think we could keep containerof() in template class, after all ListImpl methods will switch to ListNodeData.

4. [x] ListImpl::is_empty() can be removed. We can implement List::is_empty() using ListImpl.size().

5. [x] We can simplify ~List() to something like (see below why):

   while (!is_empty()) {
       pop_front();
   }

6. [x] Also we can move impl_.head.list = NULL; from ~List() to ~ListImpl(). Since ListImpl is responsible to initialize head, it's logical that it's also responsible to deinitialize it.

7. [x] After these changes we can make check_is_member() a private method of ListImpl.

8. [x] Since ListImpl is a class with methods, not a C-like struct, for consistency I suggest to make head field private and add a public getter ListNodeData& head().

[veronikaro]

Hi @gavv, sorry for the delay. I addressed all your comments, thank you very much for the feedback.

8. Since ListImpl is a class with methods, not a C-like struct, for consistency I suggest to make head field private and add a public getter `ListNodeData& head()`.

I did this at the cost of implementing push_back as a function of ListImpl level. The reason for this is that push_back requires a pointer to the head.

I would appreciate your review!

[github-actions[bot]]

:robot: The latest upstream change made this pull request unmergeable. Please resolve the merge conflicts.

[gavv]

@veronikaro Hi, my recent commit (2510edd81dbd8003d1ea0a74cecefcc0622c9534) introduced lots of conflicts with this patch, so I ported your code to the new version and finished remaining issues: bef7210761ad2ff063da7ce55e7a974a3dc9c6fd

Thanks!