Skip to content

STYLE: Remove initial << insertion from itkExceptionMacro(<< " calls#4057

Merged
dzenanz merged 1 commit intoInsightSoftwareConsortium:masterfrom
N-Dekker:Remove-literal-string-insertions-itkExceptionMacro
Jun 1, 2023
Merged

STYLE: Remove initial << insertion from itkExceptionMacro(<< " calls#4057
dzenanz merged 1 commit intoInsightSoftwareConsortium:masterfrom
N-Dekker:Remove-literal-string-insertions-itkExceptionMacro

Conversation

@N-Dekker
Copy link
Copy Markdown
Contributor

@N-Dekker N-Dekker commented May 30, 2023

Let the C++ preprocessor append the specified literal string to the error message, instead of doing the equivalent << insertion at run-time.

Aims to shorten the code a little bit, and improve consistency. The code base already had more than 750 itkExceptionMacro calls that passed a literal string directly, without having << at its left side.

@N-Dekker
STYLE: Remove initial << insertion from itkExceptionMacro(<< " calls
e254f8d 
Let the C++ preprocessor append the specified literal string to the error
message, instead of doing the equivalent `<<` insertion at run-time.

Aims to shorten the code a little bit, and improve consistency. The code base
already had more than 750 `itkExceptionMacro` calls that passed a literal
string directly, without having `<<` at its left side.
@github-actions github-actions Bot added area:Python wrapping Python bindings for a class type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Bridge Issues affecting the Bridge module area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module area:Video Issues affecting the Video module type:Style Style changes: no logic impact (indentation, comments, naming) area:Numerics Issues affecting the Numerics module labels May 30, 2023
@N-Dekker N-Dekker marked this pull request as ready for review May 31, 2023 13:09
issakomi
issakomi reviewed May 31, 2023
View reviewed changes
Comment thread Modules/IO/GDCM/src/itkGDCMImageIO.cxx
if (this->GetNumberOfComponents() != 1)
{
itkExceptionMacro(<< "Sorry Dave I can't do that");
itkExceptionMacro("Sorry Dave I can't do that");
Copy link
Copy Markdown
Member

@issakomi issakomi May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“I'm sorry, Dave. I'm afraid I can't do that,” said HAL 9000
(c) 2001: A Space Odyssey

Copy link
Copy Markdown
Contributor Author

@N-Dekker N-Dekker Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reference, Mihail! Honestly I doubt if such an error message is very helpful to the user. In general, I'm not a fan of "funny" error messages. Unless they really help to communicate to the user what's going wrong. But anyway, that's beyond the scope of this pull request!

Copy link
Copy Markdown
Member

@issakomi issakomi Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't use funny error messages too. I am not sure what this one means, just reminded me of the citation from the movie, but it may be something else.

Copy link
Copy Markdown
Member

@dzenanz dzenanz Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is an error message which is not supposed to be reachable by the GDCM's users?

Copy link
Copy Markdown
Contributor Author

@N-Dekker N-Dekker Jun 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe... but then I guess the message is only for someone named Dave 😸 Thanks for merging @dzenanz !

@dzenanz dzenanz merged commit 89beb04 into InsightSoftwareConsortium:master Jun 1, 2023
@N-Dekker
Copy link
Copy Markdown
Contributor Author

N-Dekker commented Jun 1, 2023

@N-Dekker

Aims to shorten the code a little bit

Oops, I see now:

+1,418 −1,394

Meaning that the PR has actually added more lines of code! Oops! I'm sorry 😸 !

N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jun 1, 2023
@N-Dekker
STYLE: Remove initial << from itkGenericExceptionMacro(<< " calls
f0653f8 
Follow-up to pull request InsightSoftwareConsortium#4057
commit 89beb04
"STYLE: Remove initial `<<` insertion from `itkExceptionMacro(<< "` calls"
@N-Dekker N-Dekker mentioned this pull request Jun 1, 2023
Merged
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jun 2, 2023
@N-Dekker
STYLE: Remove initial << insertion from itkDebugMacro(<< " calls
e2008ae 
Follow-up to pull request InsightSoftwareConsortium#4057
commit 89beb04
"STYLE: Remove initial `<<` insertion from `itkExceptionMacro(<< "` calls"

Aims to shorten the code a little bit, and improve consistency. The code base
already had more than 500 `itkDebugMacro` calls that passed a literal string as
first argument directly, without having `<<` at its left side.
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jun 2, 2023
@N-Dekker
STYLE: Remove initial << insertion from itkWarningMacro(<< " calls
d737e4f 
Follow-up to pull request InsightSoftwareConsortium#4057
commit 89beb04
"STYLE: Remove initial `<<` insertion from `itkExceptionMacro(<< "` calls"

Aims to shorten the code a little bit, and improve consistency. The code base
already had more than 80 `itkWarningMacro` calls that passed a literal string as
first argument directly, without having `<<` at its left side.
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jun 2, 2023
@N-Dekker
STYLE: Remove initial << from itkGenericOutputMacro(<< " calls
098c247 
Follow-up to pull request InsightSoftwareConsortium#4057
commit 89beb04
"STYLE: Remove initial `<<` insertion from `itkExceptionMacro(<< "` calls"

Aims to shorten the code a little bit, and improve consistency. The code base
already had seven `itkGenericOutputMacro` calls that passed a literal string as
first argument directly, without having `<<` at its left side.
dzenanz pushed a commit that referenced this pull request Jun 2, 2023
@N-Dekker @dzenanz
STYLE: Remove initial << from itkGenericExceptionMacro(<< " calls
1d0d43c 
Follow-up to pull request #4057
commit 89beb04
"STYLE: Remove initial `<<` insertion from `itkExceptionMacro(<< "` calls"
dzenanz pushed a commit that referenced this pull request Jun 2, 2023
@N-Dekker @dzenanz
STYLE: Remove initial << insertion from itkDebugMacro(<< " calls
b52f423 
Follow-up to pull request #4057
commit 89beb04
"STYLE: Remove initial `<<` insertion from `itkExceptionMacro(<< "` calls"

Aims to shorten the code a little bit, and improve consistency. The code base
already had more than 500 `itkDebugMacro` calls that passed a literal string as
first argument directly, without having `<<` at its left side.
dzenanz pushed a commit that referenced this pull request Jun 2, 2023
@N-Dekker @dzenanz
STYLE: Remove initial << insertion from itkWarningMacro(<< " calls
c6fedbd 
Follow-up to pull request #4057
commit 89beb04
"STYLE: Remove initial `<<` insertion from `itkExceptionMacro(<< "` calls"

Aims to shorten the code a little bit, and improve consistency. The code base
already had more than 80 `itkWarningMacro` calls that passed a literal string as
first argument directly, without having `<<` at its left side.
dzenanz pushed a commit that referenced this pull request Jun 2, 2023
@N-Dekker @dzenanz
STYLE: Remove initial << from itkGenericOutputMacro(<< " calls
30ef873 
Follow-up to pull request #4057
commit 89beb04
"STYLE: Remove initial `<<` insertion from `itkExceptionMacro(<< "` calls"

Aims to shorten the code a little bit, and improve consistency. The code base
already had seven `itkGenericOutputMacro` calls that passed a literal string as
first argument directly, without having `<<` at its left side.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
@N-Dekker @dzenanz
STYLE: Remove initial << from itkGenericExceptionMacro(<< " calls
a65fbd7 
Follow-up to pull request InsightSoftwareConsortium#4057
commit ddf899a
"STYLE: Remove initial `<<` insertion from `itkExceptionMacro(<< "` calls"
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
@N-Dekker @dzenanz
STYLE: Remove initial << insertion from itkDebugMacro(<< " calls
1c0e651 
Follow-up to pull request InsightSoftwareConsortium#4057
commit ddf899a
"STYLE: Remove initial `<<` insertion from `itkExceptionMacro(<< "` calls"

Aims to shorten the code a little bit, and improve consistency. The code base
already had more than 500 `itkDebugMacro` calls that passed a literal string as
first argument directly, without having `<<` at its left side.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
@N-Dekker @dzenanz
STYLE: Remove initial << insertion from itkWarningMacro(<< " calls
ed874ed 
Follow-up to pull request InsightSoftwareConsortium#4057
commit ddf899a
"STYLE: Remove initial `<<` insertion from `itkExceptionMacro(<< "` calls"

Aims to shorten the code a little bit, and improve consistency. The code base
already had more than 80 `itkWarningMacro` calls that passed a literal string as
first argument directly, without having `<<` at its left side.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
@N-Dekker @dzenanz
STYLE: Remove initial << from itkGenericOutputMacro(<< " calls
41f6039 
Follow-up to pull request InsightSoftwareConsortium#4057
commit ddf899a
"STYLE: Remove initial `<<` insertion from `itkExceptionMacro(<< "` calls"

Aims to shorten the code a little bit, and improve consistency. The code base
already had seven `itkGenericOutputMacro` calls that passed a literal string as
first argument directly, without having `<<` at its left side.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dzenanz dzenanz dzenanz approved these changes

+1 more reviewer

@issakomi issakomi issakomi left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area:Bridge Issues affecting the Bridge module area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Numerics Issues affecting the Numerics module area:Python wrapping Python bindings for a class area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module area:Video Issues affecting the Video module type:Style Style changes: no logic impact (indentation, comments, naming) type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@N-Dekker @issakomi @dzenanz