Skip to content

Comments

Harmonize const-ness in relation navigation return types#204

Merged
tmadlener merged 1 commit intoiLCSoft:masterfrom
dudarboh:remove_const
Mar 24, 2025
Merged

Harmonize const-ness in relation navigation return types#204
tmadlener merged 1 commit intoiLCSoft:masterfrom
dudarboh:remove_const

Conversation

@dudarboh
Copy link
Member

BEGINRELEASENOTES

  • Make getRelatedToMaxWeightAndObject(), getRelatedToMaxWeightObject(), getRelatedFromMaxWeightObject() return non-const LCObject pointer.

ENDRELEASENOTES

@dudarboh dudarboh changed the title retung non-const pointer as everywhere else retung non-const object pointers Mar 19, 2025
Copy link
Contributor

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

Is there a use case where you would need to modify the pointed-to contents? If symmetry is the main argument, I would rather add const to other methods than removing them here.

@dudarboh
Copy link
Member Author

dudarboh commented Mar 21, 2025

My goal would be that people can easily switch from getRelatedToObjects() to getRelatedToMaxWeightAndObject() by intuitively updating their code, without any extra debugging of const correctness.

For example, I switched from using

EVENT::MCParticle* getMcFromTrack(EVENT::Track* track, const UTIL::LCRelationNavigator& trk2mc){
        auto& objects = trk2mc.getRelatedToObjects(track);
        auto& weights = trk2mc.getRelatedToWeights(track);
        if ( weights.empty() ) return nullptr;
        auto maxWeightIdx = std::max_element(weights.begin(), weights.end()) - weights.begin();
        return static_cast <EVENT::MCParticle*> (objects[maxWeightIdx]);
}

to this

EVENT::MCParticle* getMcFromTrack(EVENT::Track* track, const UTIL::LCRelationNavigator& trk2mc){
    return static_cast <EVENT::MCParticle*> ( trk2mc.getRelatedToMaxWeightObject(track) );
}

and it immediately broke because if I use getRelatedToMaxWeightObject() I had to do this instead:

const EVENT::MCParticle* getMcFromTrack(EVENT::Track* track, const UTIL::LCRelationNavigator& trk2mc){
    return static_cast <const EVENT::MCParticle*> ( trk2mc.getRelatedToMaxWeightObject(track) );
}

or what I do right now to keep everything as it previously was:

EVENT::MCParticle* getMcFromTrack(EVENT::Track* track, const UTIL::LCRelationNavigator& trk2mc){
    return static_cast <EVENT::MCParticle*> ( const_cast<EVENT::LCObject*> (trk2mc.getRelatedToMaxWeightObject(track) ));
}

so I wouldn't mind switching everything to const, if we also switch the old getRelatedToObjects() to returning const.
However, then the whole MarlinReco is broken, because I assume nobody did static_cast to const, and it should be fixed everywhere these methods are used.

Copy link
Contributor

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

Thanks for the detailed explanation. Given that, I think this is indeed the quickest and easiest solution.

@tmadlener tmadlener changed the title retung non-const object pointers Harmonize const-ness in relation navigation return types Mar 24, 2025
@tmadlener tmadlener linked an issue Mar 24, 2025 that may be closed by this pull request
@tmadlener tmadlener merged commit 7a0d352 into iLCSoft:master Mar 24, 2025
12 checks passed
@dudarboh dudarboh deleted the remove_const branch March 24, 2025 15:54
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

Successfully merging this pull request may close these issues.

Removing const from the recent getRelatedToMaxWeightAndObject methods

2 participants