ENH: Add ComputeIndex() member function to ImageConstIterator

[thewtex]

Provides an alternative to ImageConstIterator::GetIndex(), making it more clear that potentially expensive computation is involved.

(cherry picked from commit 3e5a75a)

[greptile-apps]

Greptile Summary

Adds a new ComputeIndex() member function to ImageConstIterator that is marked [[nodiscard]] and makes the cost of the operation explicit in its name, then refactors GetIndex() to delegate to it with an updated doc note steering users toward the preferred API. The change is backward-compatible, correctly scoped to the ImageConstIterator hierarchy (the separate WithIndex family is unaffected), and the implementation is straightforward inline delegation.

Confidence Score: 5/5

Safe to merge; all findings are minor documentation/style issues with no correctness impact.

The logic change is trivially correct — GetIndex() simply delegates to ComputeIndex(), which contains the original one-liner body. All existing callers of GetIndex() continue to work. The two review comments are P2 style issues that do not affect runtime behavior.

No files require special attention.

Important Files Changed

Filename	Overview
Modules/Core/Common/include/itkImageConstIterator.h	Adds [[nodiscard]] ComputeIndex() and refactors GetIndex() to delegate to it; two minor P2 style issues (broken \sa SetIndex back-link, redundant const on return-by-value).

Sequence Diagram

sequenceDiagram
    participant Caller
    participant ImageConstIterator
    participant ImageBase

    Note over Caller,ImageBase: New preferred API
    Caller->>ImageConstIterator: ComputeIndex() [[nodiscard]]
    ImageConstIterator->>ImageBase: ComputeIndex(m_Offset)
    ImageBase-->>ImageConstIterator: IndexType
    ImageConstIterator-->>Caller: IndexType

    Note over Caller,ImageBase: Legacy API (now delegates)
    Caller->>ImageConstIterator: GetIndex()
    ImageConstIterator->>ImageConstIterator: ComputeIndex()
    ImageConstIterator->>ImageBase: ComputeIndex(m_Offset)
    ImageBase-->>ImageConstIterator: IndexType
    ImageConstIterator-->>Caller: const IndexType

Loading

_{Reviews (1): Last reviewed commit: "ENH: Add `ComputeIndex()` member functio..." | Re-trigger Greptile}

[greptile-apps]

Missing \sa SetIndex cross-reference breaks bidirectional Doxygen link

SetIndex() still cross-references GetIndex() (\sa GetIndex), but after this change GetIndex() only cross-references ComputeIndex() — the reciprocal pointer to SetIndex is gone. Consider adding it back:

Suggested change

	/** Computes and returns the index. This may be a relatively expensive operation.
	* \note It is often preferable for users to call ComputeIndex() directly, to make it more clear that this function
	* may be expensive.
	* \sa ComputeIndex */
	const IndexType
	GetIndex() const
	{
	return m_Image->ComputeIndex(static_cast<OffsetValueType>(m_Offset));
	return this->ComputeIndex();
	}
	/** Computes and returns the index. This may be a relatively expensive operation.
	* \note It is often preferable for users to call ComputeIndex() directly, to make it more clear that this function
	* may be expensive.
	* \sa ComputeIndex
	* \sa SetIndex */
	const IndexType
	GetIndex() const
	{
	return this->ComputeIndex();
	}

[greptile-apps]

const return-by-value inconsistency with ComputeIndex()

GetIndex() returns const IndexType while the new ComputeIndex() (which it wraps) returns plain IndexType. A top-level const on a return-by-value type is discarded by the type system and can inhibit move semantics, so the two signatures are inconsistent without benefit. Since GetIndex() is the legacy entry point this is pre-existing, but the asymmetry is now more visible next to the preferred ComputeIndex():

Suggested change

	const IndexType
	GetIndex() const
	IndexType
	GetIndex() const

[N-Dekker]

Thanks, Matt 👍