Within OptionsDialog.cpp, only call DrivesToolbarRefreshAllIcons() if the setting actually changed.
Code: Select all
sDisplayName << szDisplayName << TEXT(" (") << szVolume << TEXT(")");
std::wstring wDisplayName = sDisplayName.str();
size_t tSize = (wDisplayName.length() + 1) * sizeof(TCHAR);
memcpy(szDisplayName, wDisplayName.c_str(), tSize);
Far too complicated for what it accomplishes. The same thing could be done simply as:
Code: Select all
Overall, I'm all for greater use of C++ within the codebase, but in the interests of keeping things consistent, I think C should be preferred in some of the older files, such as DrivesToolbarHandler.cpp. This is especially true when the equivalent C code would be simpler and shorter than the C++ code.
Also, a large chunk of the code within InsertDriveIntoDrivesToolbar() is the same as the code in InsertDriveIntoDrivesToolbar(). Find a way to extract the common functionality.
Finally, I feel I should say the comments in this post and the two above are not meant to be critical. They're more my blunt assessment of the code. I know I've probably done exactly the same types of things as above in portions of the code, but you have to keep in mind that Explorer++ was started in 2005, and parts of the code are quite old.
My views on it are:
1. I impose stricter standards on myself now than I once did. That either means I try not to commit "bad" code, or I refactor it shortly afterwards.
2. I impose even stricter standards on code submitted by other people. Mainly because I'm not sure if they'll be around to refactor it; in all honesty, I'll probably be the person that has to maintain the code, which means I want it to be as good as possible when its first committed.
Nonetheless, thanks for the work you've both done so far Michael and Ryan, it is appreciated.