Tectu / cpp-properties

A library that brings C# like properties to modern C++.
https://cppproperties.insane.engineer
MIT License
23 stars 4 forks source link

review of the docs @ https://cppproperties.insane.engineer/ #7

Open bjornstromberg opened 7 months ago

bjornstromberg commented 7 months ago

a review as promised in gpds repo issue

this is very subjective since this is my view of what i expect from a projects documentation, and ofcourse there are different sizes, but if we never fix the questions, we will never get the right answers. if the answer is Unknown, then document that.

so the initial frontpage 'overview' information I'm looking for from a library documentation (on a website or pdf generated) is:

ofcourse after that we got the deeper information, this part is to be viewed from two perspectives

  1. a how do i use this library? a few full examples that should be runable to show the basic functions/features, should be supplied as code in the repo, then a quick runthrough of the examples in the documentation.
  2. a pure reference of the supplied public interfaces of the library.

from this list there is alot of missing information on the testpage, but as with everything, nothing starts out as done unless its a extremely small task.

a project that starts out as a small simple thing, might escalate and become very large.

from my knowledge the amount of projects that actually meet my critera of how a good documentation looks is about '0', not even my own internal projects meet that critera due to the massive effort it takes to update and the docs fast get out of sync with reality.

but it can alteast be a roadmap of what kind of information is wanted.

for a starter we got the same issue as with gpds with the external tinyxml2 and pkgconf, so i just disabled the tinyxml2 dependency on cpp-properties, since gpds should in my mind supply the missing xml support on my system since we fixed that.

next post will be made in a few hours when i have had time to do the legwork..

bjornstromberg commented 7 months ago

ps from https://gohugo.io/ it seems that this documentation project actually might be worth having a look at, so thanks for telling me about it.

bjornstromberg commented 7 months ago

one quick thing, earlier i started a compilation and went from the computer, tinyxml2 linking seems to be a hard dependency on the serialization even though gpds should bring the library as its dependency.

things like these are the reason for my somewhat extreme view on documentation

/home/user/Build/docs-review-tectu-cpp-properties> /usr/bin/ninja
[6/26] Building CXX object test/CMakeFiles/tests.dir/test_suites/properties.cpp.o
[7/26] Building CXX object test/CMakeFiles/tests.dir/test_suites/attributes.cpp.o
[8/26] Building CXX object test/CMakeFiles/tests.dir/test_suites/02_builtin_types_cpp.cpp.o
[9/26] Building CXX object test/CMakeFiles/tests.dir/test_suites/01_builtin_types.cpp.o
[10/26] Building CXX object examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o
[11/26] Building CXX object examples/notification/CMakeFiles/example-notification.dir/main.cpp.o
[12/26] Building CXX object examples/nested/CMakeFiles/example-nested.dir/main.cpp.o
[13/26] Building CXX object examples/linked_property_functions/CMakeFiles/example-linkedpropertyfunctions.dir/main.cpp.o
[14/26] Building CXX object examples/linked_properties/CMakeFiles/example-linkedproperties.dir/main.cpp.o
[15/26] Linking CXX executable examples/serialization/example-serialization
ninja: job failed: : && /usr/bin/clang++ -O2 -g -DNDEBUG  examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o -o examples/serialization/example-serialization   && :
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tct::properties::archiver_xml::save[abi:cxx11](tct::properties::properties const&) const':
/home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/main.cpp:58:(.text._ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE[_ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE]+0x36): undefined reference to `tinyxml2::XMLDocument::XMLDocument(bool, tinyxml2::Whitespace)'
/usr/bin/ld: /home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/main.cpp:61:(.text._ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE[_ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE]+0x45): undefined reference to `tinyxml2::XMLDocument::NewElement(char const*)'
/usr/bin/ld: /home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/main.cpp:62:(.text._ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE[_ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE]+0x58): undefined reference to `tinyxml2::XMLNode::InsertEndChild(tinyxml2::XMLNode*)'
/usr/bin/ld: /home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/main.cpp:68:(.text._ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE[_ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE]+0x7b): undefined reference to `tinyxml2::XMLPrinter::XMLPrinter(_IO_FILE*, bool, int)'
/usr/bin/ld: /home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/main.cpp:69:(.text._ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE[_ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE]+0x8d): undefined reference to `tinyxml2::XMLDocument::Print(tinyxml2::XMLPrinter*) const'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tct::properties::archiver_xml::save[abi:cxx11](tct::properties::properties const&) const':
/home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:73:(.text._ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE[_ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE]+0xaf): undefined reference to `tinyxml2::XMLDocument::Clear()'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tinyxml2::XMLPrinter::~XMLPrinter()':
/usr/include/tinyxml2.h:2247:(.text._ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE[_ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE]+0xb6): undefined reference to `vtable for tinyxml2::XMLPrinter'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tct::properties::archiver_xml::save[abi:cxx11](tct::properties::properties const&) const':
/home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:76:(.text._ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE[_ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE]+0x104): undefined reference to `tinyxml2::XMLDocument::~XMLDocument()'
/usr/bin/ld: /home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:76:(.text._ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE[_ZNK3tct10properties12archiver_xml4saveB5cxx11ERKNS0_10propertiesE]+0x160): undefined reference to `tinyxml2::XMLDocument::~XMLDocument()'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tct::properties::archiver_xml::write_recursively(tinyxml2::XMLDocument&, tinyxml2::XMLElement&, tct::properties::properties const&)':
/home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:131:(.text._ZN3tct10properties12archiver_xml17write_recursivelyERN8tinyxml211XMLDocumentERNS2_10XMLElementERKNS0_10propertiesE[_ZN3tct10properties12archiver_xml17write_recursivelyERN8tinyxml211XMLDocumentERNS2_10XMLElementERKNS0_10propertiesE]+0x66): undefined reference to `tinyxml2::XMLNode::InsertEndChild(tinyxml2::XMLNode*)'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tinyxml2::XMLElement::SetAttribute(char const*, char const*)':
/usr/include/tinyxml2.h:1466:(.text._ZN3tct10properties12archiver_xml17write_recursivelyERN8tinyxml211XMLDocumentERNS2_10XMLElementERKNS0_10propertiesE[_ZN3tct10properties12archiver_xml17write_recursivelyERN8tinyxml211XMLDocumentERNS2_10XMLElementERKNS0_10propertiesE]+0x12c): undefined reference to `tinyxml2::XMLElement::FindOrCreateAttribute(char const*)'
/usr/bin/ld: /usr/include/tinyxml2.h:1467:(.text._ZN3tct10properties12archiver_xml17write_recursivelyERN8tinyxml211XMLDocumentERNS2_10XMLElementERKNS0_10propertiesE[_ZN3tct10properties12archiver_xml17write_recursivelyERN8tinyxml211XMLDocumentERNS2_10XMLElementERKNS0_10propertiesE]+0x137): undefined reference to `tinyxml2::XMLAttribute::SetAttribute(char const*)'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tct::properties::archiver_xml::write_recursively(tinyxml2::XMLDocument&, tinyxml2::XMLElement&, tct::properties::properties const&)':
/home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:110:(.text._ZN3tct10properties12archiver_xml17write_recursivelyERN8tinyxml211XMLDocumentERNS2_10XMLElementERKNS0_10propertiesE[_ZN3tct10properties12archiver_xml17write_recursivelyERN8tinyxml211XMLDocumentERNS2_10XMLElementERKNS0_10propertiesE]+0x176): undefined reference to `tinyxml2::XMLDocument::NewElement(char const*)'
/usr/bin/ld: /home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:124:(.text._ZN3tct10properties12archiver_xml17write_recursivelyERN8tinyxml211XMLDocumentERNS2_10XMLElementERKNS0_10propertiesE[_ZN3tct10properties12archiver_xml17write_recursivelyERN8tinyxml211XMLDocumentERNS2_10XMLElementERKNS0_10propertiesE]+0x1d1): undefined reference to `tinyxml2::XMLElement::SetText(char const*)'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tinyxml2::XMLElement::SetAttribute(char const*, char const*)':
/usr/include/tinyxml2.h:1466:(.text._ZN3tct10properties12archiver_xml17write_recursivelyERN8tinyxml211XMLDocumentERNS2_10XMLElementERKNS0_10propertiesE[_ZN3tct10properties12archiver_xml17write_recursivelyERN8tinyxml211XMLDocumentERNS2_10XMLElementERKNS0_10propertiesE]+0x29c): undefined reference to `tinyxml2::XMLElement::FindOrCreateAttribute(char const*)'
/usr/bin/ld: /usr/include/tinyxml2.h:1467:(.text._ZN3tct10properties12archiver_xml17write_recursivelyERN8tinyxml211XMLDocumentERNS2_10XMLElementERKNS0_10propertiesE[_ZN3tct10properties12archiver_xml17write_recursivelyERN8tinyxml211XMLDocumentERNS2_10XMLElementERKNS0_10propertiesE]+0x2a7): undefined reference to `tinyxml2::XMLAttribute::SetAttribute(char const*)'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tinyxml2::XMLPrinter::~XMLPrinter()':
/usr/include/tinyxml2.h:2247:(.text._ZN8tinyxml210XMLPrinterD2Ev[_ZN8tinyxml210XMLPrinterD2Ev]+0x7): undefined reference to `vtable for tinyxml2::XMLPrinter'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tct::properties::archiver_xml::load(tct::properties::properties&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const':
/home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/main.cpp:83:(.text._ZNK3tct10properties12archiver_xml4loadERNS0_10propertiesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK3tct10properties12archiver_xml4loadERNS0_10propertiesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0x38): undefined reference to `tinyxml2::XMLDocument::XMLDocument(bool, tinyxml2::Whitespace)'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tct::properties::archiver_xml::load(tct::properties::properties&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const':
/home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:84:(.text._ZNK3tct10properties12archiver_xml4loadERNS0_10propertiesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK3tct10properties12archiver_xml4loadERNS0_10propertiesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0x4a): undefined reference to `tinyxml2::XMLDocument::Parse(char const*, unsigned long)'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tinyxml2::XMLNode::FirstChildElement(char const*)':
/usr/include/tinyxml2.h:783:(.text._ZNK3tct10properties12archiver_xml4loadERNS0_10propertiesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK3tct10properties12archiver_xml4loadERNS0_10propertiesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0x5b): undefined reference to `tinyxml2::XMLNode::FirstChildElement(char const*) const'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tct::properties::archiver_xml::load(tct::properties::properties&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const':
/home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:95:(.text._ZNK3tct10properties12archiver_xml4loadERNS0_10propertiesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK3tct10properties12archiver_xml4loadERNS0_10propertiesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0xde): undefined reference to `tinyxml2::XMLDocument::~XMLDocument()'
/usr/bin/ld: /home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:95:(.text._ZNK3tct10properties12archiver_xml4loadERNS0_10propertiesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK3tct10properties12archiver_xml4loadERNS0_10propertiesERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0x114): undefined reference to `tinyxml2::XMLDocument::~XMLDocument()'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tct::properties::archiver_xml::read_recursively(tinyxml2::XMLElement&, tct::properties::properties&)':
/home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/main.cpp:141:(.text._ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE[_ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE]+0x71): undefined reference to `tinyxml2::XMLAttribute::Name() const'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tct::properties::archiver_xml::read_recursively(tinyxml2::XMLElement&, tct::properties::properties&)':
/home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:141:(.text._ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE[_ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE]+0xfd): undefined reference to `tinyxml2::XMLAttribute::Value() const'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tinyxml2::XMLNode::FirstChildElement(char const*)':
/usr/include/tinyxml2.h:783:(.text._ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE[_ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE]+0x22c): undefined reference to `tinyxml2::XMLNode::FirstChildElement(char const*) const'
/usr/bin/ld: examples/serialization/CMakeFiles/example-serialization.dir/main.cpp.o: in function `tct::properties::archiver_xml::read_recursively(tinyxml2::XMLElement&, tct::properties::properties&)':
/home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:150:(.text._ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE[_ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE]+0x23c): undefined reference to `tinyxml2::XMLElement::GetText() const'
/usr/bin/ld: /home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:161:(.text._ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE[_ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE]+0x249): undefined reference to `tinyxml2::XMLElement::GetText() const'
/usr/bin/ld: /home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:162:(.text._ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE[_ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE]+0x25a): undefined reference to `tinyxml2::XMLElement::GetText() const'
/usr/bin/ld: /home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:164:(.text._ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE[_ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE]+0x30a): undefined reference to `tinyxml2::XMLElement::GetText() const'
/usr/bin/ld: /home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:169:(.text._ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE[_ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE]+0x3e5): undefined reference to `tinyxml2::XMLAttribute::Name() const'
/usr/bin/ld: /home/user/Develop/docs-review-tectu-cpp-properties/examples/serialization/../../cppproperties/archiver_xml.hpp:169:(.text._ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE[_ZN3tct10properties12archiver_xml16read_recursivelyERN8tinyxml210XMLElementERNS0_10propertiesE]+0x46d): undefined reference to `tinyxml2::XMLAttribute::Value() const'
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: subcommand failed
*** Failure: Exit code 1 ***

since it wont even compile i wont waste time on trying to fix issues, since this is a testing repo.

I'll do a review on the page and just browse the code in github instead

bjornstromberg commented 7 months ago

to start with, you know my position on macros, so i wont comment that on every occation.

bjornstromberg commented 7 months ago

i've had a look at the raw properties https://cppproperties.insane.engineer/docs/properties/raw_properties/#raw-properties but by now im confused, now were back at using structs, which makes the whole thing hard to grasp from just reading the manual and the code, by here i would need to either run the code, and be able to test and experiment with it, or get some input from you on how it is supposed to work.

going on with the pages, the code snippets show hardcoded functions for get and set etc so I'm starting to think this is something entirely different from what i thought it should do...

this is a reflection thing so via an interface the data from the object, that the other side does not even know its true static typed type of, can extract that data. and here i thought it was going to facilitate less code to write from the first pages.

serialization is good, it seems to do what i would expect from it, could show to actually serialize and deserialize it in a quick snippet of a roundtrip into a different object.

the qt widgets plugin page is also kind of lacking, we both now how much trouble you can get into with memleaks of raw unparented widgets, and what happens with shared pointers and child widgets in shared pointers when they double delete and such, and information how this kind of issues are handled is lacking.

it takes alot of accounting behind the scenes, if i where to use a library in production code, i want to know these issues are handled or if i need to handle them (the resulting widget is a raw pointer..)

in this case it is a std::unique_ptr that is returned, which gives a possible problem with children, it would more or less require some kind of custodian to handle the fact that the child needs to be parentless when it is destroyed. it also has the problem that if someone use deleteLater() on that object somewhere, that interface the object, your up the famous creek without a paddle..

bjornstromberg commented 7 months ago

and lastly the github link (it points correctly) and the author link to https://silizium.io/ which redirects to https://blog.insane.engineer/ and shows a ssl warning on the certificate.

so not sure what more i can do without some input from you.

have a nice day!