Skip to content

STYLE: Replace Allocate(); FillBuffer({}) with AllocateInitialized()#5582

Merged
dzenanz merged 1 commit intoInsightSoftwareConsortium:mainfrom
N-Dekker:Replace-Allocate-FillBuffer-with-AllocateInitialized
Oct 31, 2025
Merged

STYLE: Replace Allocate(); FillBuffer({}) with AllocateInitialized()#5582
dzenanz merged 1 commit intoInsightSoftwareConsortium:mainfrom
N-Dekker:Replace-Allocate-FillBuffer-with-AllocateInitialized

Conversation

@N-Dekker
Copy link
Copy Markdown
Contributor

Replaced lines of code of the form

image->Allocate();
image->FillBuffer(PixelType{});

With image->AllocateInitialized();.

Using Notepad++, Replace in Files, doing:

Find what: (.+->)Allocate\(\);[\r\n]+\1FillBuffer\(.*{}\);
Replace with: $1AllocateInitialized\(\);

@github-actions github-actions Bot added type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Filtering Issues affecting the Filtering module area:Segmentation Issues affecting the Segmentation module type:Style Style changes: no logic impact (indentation, comments, naming) labels Oct 30, 2025
Replaced lines of code of the form

    image->Allocate();
    image->FillBuffer(PixelType{});

With `image->AllocateInitialized();`.

Using Notepad++, Replace in Files, doing:

  Find what: `^(  .+->)Allocate\(\);[\r\n]+\1FillBuffer\(.*{}\);`
  Replace with: `$1AllocateInitialized\(\);`

Manually replaced a few more cases, found by the regular expression
`FillBuffer\(.*\{\}\);`

Follow-up to pull request InsightSoftwareConsortium#4494
commit cd49925
"STYLE: Replace `Allocate(); FillBuffer(0)` with `AllocateInitialized()`"
@N-Dekker N-Dekker force-pushed the Replace-Allocate-FillBuffer-with-AllocateInitialized branch from 52b4159 to b37c81b Compare October 31, 2025 10:38
Copy link
Copy Markdown
Member

@hjmjohnson hjmjohnson left a comment

Choose a reason for hiding this comment

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

Very nice. Approved when CI passes.

@N-Dekker N-Dekker marked this pull request as ready for review October 31, 2025 12:53
@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Oct 31, 2025

Are there any instances of AllocateInitialized() soon followed by FillBuffer()? If so, we could turn those into Allocate(false) to avoid double initialization.

@N-Dekker
Copy link
Copy Markdown
Contributor Author

Are there any instances of AllocateInitialized() soon followed by FillBuffer()? If so, we could turn those into Allocate(false) to avoid double initialization.

Do you mean, specifically when FillBuffer(fillValue) is called with a non-zero fill-value?

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Oct 31, 2025

Yes.

@N-Dekker
Copy link
Copy Markdown
Contributor Author

I think there are still a few unnecessary FillBuffer(<zero>) calls that I would like to address, once this PR is processed. (With this PR I only addressed fill values specified by braces, {}.) Once these are addressed, yes, let's look at unnecessary double initializations 👍

@dzenanz dzenanz merged commit eb4386f into InsightSoftwareConsortium:main Oct 31, 2025
17 checks passed
@N-Dekker
Copy link
Copy Markdown
Contributor Author

N-Dekker commented Nov 2, 2025

Are there any instances of AllocateInitialized() soon followed by FillBuffer()? If so, we could turn those into Allocate(false) to avoid double initialization.

@dzenanz No instances found, using Notepad++ Find in Files, with the following regular expression: ^( .+)AllocateInitialized\(\);[\r\n]+\1FillBuffer Did you see any instance like that?

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Nov 3, 2025

I didn't, I was just wondering if there are any.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Filtering Issues affecting the Filtering module area:Segmentation Issues affecting the Segmentation 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

Development

Successfully merging this pull request may close these issues.

3 participants