-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ADD] Debug Function with Variadic Functions #26
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @rajivnayanc - good additions. Looks neat, and thank you for changing all the macros to using variadic templates. I have suggested a few changes, which are mostly nits. Approving the PR but will appreciate if those nits can be addressed before we merge this.
Thank you for your contribution. 👏🏻 🎉
build.sh
Outdated
cmake .. | ||
make |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think having cmake .. && make
makes more sense? If any one of them fails, then the other should ideally not run as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I have updated this in next commit.
void debug(); | ||
template <typename T, typename... Types> | ||
void debug(T var1, Types... var2); | ||
|
||
// Variadic Debug Function with new line at the end | ||
void debugln(); | ||
template <typename T, typename... Types> | ||
void debugln(T var1, Types... var2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
@@ -32,16 +32,15 @@ FileManager::file_info FileManager::make_file_info(std::string filename, std::st | |||
std::vector<FileManager::file_info> FileManager::list_files(std::vector<std::string> extensions, bool ignore_extensions) { | |||
// This returns the list of files present in the folder: corePath | |||
// TODO: Add tests, check if corePath is not empty | |||
// Converting #ifdef DEBUG and #endif to a macro |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for being thorough.
std::cout << "Listing files and dirs in: " << corePath << "\n"; | ||
#endif | ||
|
||
debugln("Listing files and dirs in:", corePath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should this be: "dirs in: "
instead of "dirs in:
(note the extra space at the end)? Or does the variadic template adds spaces at the end every time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variadic template adds a space between two variables every time.
continue; | ||
} else { | ||
this->clear(entry.name); | ||
this->writeToFileIterated(file, depth, ignore_dirs, ignore_extensions); | ||
} | ||
} | ||
} | ||
debugln("Done!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we missing ifdef
and #endif
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, #ifdef
and #endif
are being added in the variadic template itself so that the code looks cleaner here.
src/FileManager.cpp
Outdated
#ifdef DEBUG | ||
std::cout << "Done!\n"; | ||
std::cout << var1 << " " ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: let's remove space just before semicolon.
src/FileManager.cpp
Outdated
|
||
// Variadic Debug Function with new line at the end | ||
void debugln() { | ||
std::cout<<"\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: let's add spaces here: std::cout << "\n";
instead of std::cout<<"\n";
.
src/FileManager.cpp
Outdated
template <typename T, typename... Types> | ||
void debugln(T var1, Types... var2) { | ||
#ifdef DEBUG | ||
std::cout << var1 << " " ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same nit as above: let's remove space before semicolon here.
I have updated the code wherever it was applicable. Please have a look. Thanks for the suggestions. cc @krshrimali |
Thanks, @rajivnayanc for the contribution. LGTM. 🚀 |
Thank You! Looking forward to make more contributions. :) |
This commit adds two variadic functions based debugger, which helps in better readability of the code. One functions end with a newline, and one doesn't. In the functions, we can pass any number of arguments to the functions that will print them space-separated when DEBUG Flag is defined. Also, build.sh is added for easily move library to samples folder.