added helpCommand() and HELP_TEXT.#23
Conversation
NetanelPa
left a comment
There was a problem hiding this comment.
As an overall this pretty goood and way more organized. You have some wrong indents (probably) and magic numbers but overall pretty good. The most important thing is the operator overloading if you don't understand something fell free to ask
|
|
||
| // prints the host vector `hosts` in a numbered list. | ||
| // only prints IPv4 and MAC addresses. | ||
| void HandleOutput::printHostVectorNumberedList(std::vector<Host> hosts) { |
There was a problem hiding this comment.
This should get the host list as it is (I explained why in line 44 - 52 suggestion)
| void HandleOutput::printHostVectorNumberedList(std::vector<Host> hosts) { | |
| void HandleOutput::printHostVectorNumberedList(const HostList& hosts) { |
There was a problem hiding this comment.
Why get a HostList as an arguement? you would need to create a HostList object for that to work, and we agreed upon using HostLists methods and fields statically
| for (const auto &h : hosts) { | ||
| std::cout << std::left | ||
| << std::setw(4) << i << RESET | ||
| << GREEN | ||
| << std::setw(15) << h.getIPv4() | ||
| << std::setw(20) << h.getMAC()<< RESET | ||
| << '\n'; | ||
| i++; | ||
| } |
There was a problem hiding this comment.
All of the printing should be with operator overloading as eli started doing that's way more sorted and readable you should also add the index varlible to there.
| for (const auto &h : hosts) { | |
| std::cout << std::left | |
| << std::setw(4) << i << RESET | |
| << GREEN | |
| << std::setw(15) << h.getIPv4() | |
| << std::setw(20) << h.getMAC()<< RESET | |
| << '\n'; | |
| i++; | |
| } | |
| std::cout << hosts; | |
| } |
There was a problem hiding this comment.
how should this be done in your opinion? overload << in the Host class or in the HostList class?
There was a problem hiding this comment.
Both in host you should print the class members and in host list you should print each host with the already overloaded operator
There was a problem hiding this comment.
this method is used in startChooseHostsLoop(). it prints a std::vector<Host> which represents new hosts found on the users LAN. the HostList class represents the hosts saved to serverinfo.json by the user, so overloading << doesnt fit the usecase here.
nevertheless i did add the oprator<< overloading to the Host and HostList classes, just because it seems like a better way to print the hosts saved by the user.
added the help text command