AddressBook: new address book entry container
coneiric opened this issue · 5 comments
By submitting this issue, I confirm the following:
- I have read and understood the contributor guide in kovri-docs.
- I have checked that the issue I am reporting can be replicated or that the feature I am suggesting is not present.
- I have checked opened or recently closed pull requests for existing solutions/implementations to my issue/suggestion.
Add a container class for individual address book entry (host-address) pairs.
Will create an interface for passing individual references or collections to/from AddressBook. New class will be similar to AddressBookEntry, generalized to be useful in more contexts.
similar to
If you're going to use public private class access modifiers and not treat the class as a POD type, then use as class and not a struct. https://github.com/monero-project/kovri/blob/master/tests/unit_tests/client/address_book/impl.cc#L49
not treat the class as a POD type
I'm not opposed to treating it as a POD type. That would make porting easier, IIUC.
For simple classes/structs like this, is that a better general design?
is that a better general design?
Basic C++. Lookup what POD means.
what POD means
Right, I was more asking about if it was worth it to convert AddressBook::Entry to a POD type.
Short answer: no.
Tried some unfruitful experiments, and it's just cleaner as a class. It just looked too heavy with the extra code to convert kovri::core::IdentHash + std::string to trivial types (and back).
Added a couple move constructors (for creating entries in different contexts), and it should be PR-ready early next week. FWIW, class definition in the topic branch is here: https://github.com/coneiric/kovri/blob/address-book-entry/src/client/address_book/impl.h#L67