resizablelib icon indicating copy to clipboard operation
resizablelib copied to clipboard

Trailing blanks and other minor things

Open irwir opened this issue 1 year ago • 9 comments

~~1. The current version got a lot of white space at line ends in Docs and Doxyfile. Global replace with regular expression can be used or some kind of trim utility.~~

~~2. In ResizableMsgSupport.h lines 80, 86 and 92 can get const modifier, lke this: inline BOOL Send_QueryProperties(HWND hWnd, const LPRESIZEPROPERTIES pResizeProperties)~~

  1. Ordering class members by size in descending order usually helps to avoid memory wasted for alignment. For example, class CResizableMinMax in ResizableMinMax.h in 64-bit build needs 4-byte padding after the last BOOL It would be better to move the block of BOOL members after POINT block - no wasted space inside the class structure. By the way, it is possible to visualise memory layout in the latest VS2022.

irwir avatar Aug 05 '24 16:08 irwir

  1. Doxyfile is automatically generated and then modified/updated. I did not insert any whitespace at the end of lines on purpose, that is done by Doxygen tools. I don't think I'll manually remove it, unless Doxygen does it automatically when I update the file.
  2. Those pointers need to be writeable, as the recipient of the message will modify the structure.
  3. That's nice, I will look into it.

Thanks

ppescher avatar Aug 05 '24 16:08 ppescher

2. Those pointers need to be writeable, as the recipient of the message will modify the structure.

The pointer does not need to be writable because constant pointer is not a pointer to a constant structure.

irwir avatar Aug 05 '24 17:08 irwir

  1. Doxyfile is automatically generated and then modified/updated. I did not insert any whitespace at the end of lines on purpose, that is done by Doxygen tools. I don't think I'll manually remove it, unless Doxygen does it automatically when I update the file.

Timmed Doxyfile gets 0.5K lighter; sources probably have no trailing blanks. For example, to trim white space in source files Crypto++ library simply runs sed in make file. Presumably after Doxygen's invokation it might be possible to run the same sed or a powershell script for all files before making a Git commit. Alternatively, it could be done manually from Visual Studio with global replace.

irwir avatar Aug 05 '24 18:08 irwir

There is only 1 whitespace on some lines that continue after the line brake, and they're all comments. I don't understand what do we gain if we remove that whitespace, it does not end up anywhere in the output. Do you mean that there is whitespace in source files that end up in the generated documentation? Does it break formatting? If not, who cares? :)

ppescher avatar Aug 05 '24 18:08 ppescher

Trailing blanks tend to appear unnoticed, and then mess file comparison. Comparison tools might have issues with differently sized files, or with showing clearly those blank spaces - then compare settings should be changed, or even other tools used. In the long run it is easier to keep everything trimmed. That is why I like approach of Crypto++ library where everything is trimmed automatically. Then it is trivial to apply trimming to some more files.

By the way, is this correct in Doxyfile or should be the same as the current library version? PROJECT_NUMBER = 1.4

irwir avatar Aug 05 '24 19:08 irwir

It seems the trailing blanks in generated files do have certain logic. Just ignore the first point in the opening message. Points 2 and 3 are still valid.

irwir avatar Aug 27 '24 16:08 irwir

I still don't understand what's the purpose of declaring a const argument to a function: arguments are always "read-only" for the caller. If you make them const you just prevent the function to use them as variables inside the function body. I don't see any gain in this. Also I don't see many APIs or runtime functions where the arguments are const values, the only thing that is actually used and useful is when you have a pointer to const or a const reference.

ppescher avatar Aug 27 '24 16:08 ppescher

Also I don't see many APIs or runtime functions where the arguments are const value

Are you sure? Open your own ResizableState.cpp file ; all LPCTSTR arguments are constant pointers to TCHAR. Declaring something const explicitly states that the thing should never change. It diagnoses possible bugs of attempted modification and gives a hint to compiler for possible optimisations.

irwir avatar Aug 27 '24 17:08 irwir

Yes, I'm quite sure I never saw a function argument declared as const LPCTSTR or char* const like you suggest...

You said that declaring const LPRESIZEPROPERTIES is equivalent to RESIZEPROPERTIES* const and not const RESIZEPROPERTIES*. The latter is not acceptable because messages can modify the content of the RESIZEPROPERTIES structure. And my point is that the first does not bring any advantage.

ppescher avatar Aug 28 '24 08:08 ppescher