Skip to content
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

Trailing blanks and other minor things #34

Open
irwir opened this issue Aug 5, 2024 · 9 comments
Open

Trailing blanks and other minor things #34

irwir opened this issue Aug 5, 2024 · 9 comments

Comments

@irwir
Copy link
Contributor

irwir commented Aug 5, 2024

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.
@ppescher
Copy link
Owner

ppescher commented Aug 5, 2024

  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

@irwir
Copy link
Contributor Author

irwir commented Aug 5, 2024

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
Copy link
Contributor Author

irwir commented Aug 5, 2024

  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.

@ppescher
Copy link
Owner

ppescher commented Aug 5, 2024

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? :)

@irwir
Copy link
Contributor Author

irwir commented Aug 5, 2024

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
Copy link
Contributor Author

irwir commented Aug 27, 2024

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.

@ppescher
Copy link
Owner

ppescher commented Aug 27, 2024

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.

@irwir
Copy link
Contributor Author

irwir commented Aug 27, 2024

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.

@ppescher
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants