schm icon indicating copy to clipboard operation
schm copied to clipboard

PList modernized

Open bo0ts opened this issue 14 years ago • 2 comments

Hi there, I like your project, but the current state of the code is inappropriate, even for a hobby project. The two commits refactor PList and annotate it with comments so you can use it as a base to explore the world of modern C++. They don't touch the algorithms although they seem very messy and inefficient. The first thing you should look at is const-correctness and references. Watch out for include guards and you have a using directive in one of your headers polluting the global scope. After that have a look at the header of the stdlib and familiarize yourself with iterators.

There are some more advanced things in this commit: move semantics (a.k.a. &&) and defaulted constructors. You can safely ignore them for now but you should read something about constructor initializer lists to write proper destructors.

The largest flaw by far was the destructor. vector frees its resources on its destruction. No need for you to do it.

Have a look at stackoverflow.com for some great resources. If you have any questions, don't hesitate to send me a message.

Cheers, Philipp

bo0ts avatar Dec 07 '11 00:12 bo0ts

Thank you Philipp,

I've removed the destructor for PList.

As for the other proposed changes I will look into them soon (I'm busy with a different project now).

I've noticed that you suggest the use of -std=c++0x , while using some of the C++11 additions to the language is appealing to me I will keep for now the code compatible with C++98. I want this to be 100% portable on all major operating systems.

Sol

sol-prog avatar Dec 14 '11 01:12 sol-prog

C++11 is already well supported by clang, gcc and MSVC. Any other mayor platforms in mind? Use boost.config to enable the features selectively. You should use cmake if you truly want something portable.

bo0ts avatar Dec 14 '11 10:12 bo0ts