Optimization#20
Conversation
Add Compiler flags -g -O3 to MakeFile Ignore build and bin directories Add performance.txt
Remove parameter uint32_t block from SHA1::blk, SHA1::R0, SHA1::R1, SHA1::R2, SHA1::R3, SHA1::R4 Remove parameters from SHA1::transform and SHA1::buffer_to_block, SHA1::reset
vog
left a comment
vog
left a comment
There was a problem hiding this comment.
All in all, very good work. Thanks a lot! My review mostly consists of minor change requests, complaining on a high level.
| * sha1.final(); | ||
| * is the same than | ||
| * sha1.update(str1 + str2 + str3); | ||
| * sha1.final(); |
There was a problem hiding this comment.
The above multi-line comment describes the usage of multiple functions in combination. Wouldn't a sub section in README.md be a better place for that?
There was a problem hiding this comment.
Most of the times, users of a library will check the headers for documentation. Also consider that many times users only download the headers and forget where it comes from.
This is only what I consider a reason for the comments to stay in the header, but if you still want to move this and the next two comments to the README, that is fine.
There was a problem hiding this comment.
I see why you proposed it that way, but would still prefer to have it in the README.
There was a problem hiding this comment.
Thanks again for your efforts. This is still highly appreciated!
| * | ||
| * @return return the sha1sum of the string (or strings) | ||
| * fed by calls to SHA1::update. | ||
| * */ |
There was a problem hiding this comment.
This multi-line comment is just a repetition of the above comment (the one that should go into the README). So this can and should be removed, to avoid becoming stale (and as time passes, perhaps even incorrect).
There was a problem hiding this comment.
The previous comment (for SHA1::update) demonstrates the usage for SHA1::update, and this comment makes explicit that SHA1::final should be used only after the last SHA1::update. (See previous comment)
| * | ||
| * @return return the sha1sum of the content of the specified file. | ||
| * | ||
| * */ |
There was a problem hiding this comment.
Please move this comment to a more visible place in the README (as with the above comments).
There was a problem hiding this comment.
See one of my previous comments.
2 participants
Encapsulate static variables and functions into the SHA1 class. (see #17)
Add:
std::string SHA1::operator()(const char* str);std::string SHA1::operator()(const std::string& str);bool SHA1::operator()(const char* fileName, std::string& hashSum);bool SHA1::operator()(const std::string& fileName, std::string& hashSum);std::string SHA1::getError();