Necessary Maintenance #18136
Fix design flaw in geo::ChannelMapAlg interface
0%
Description
geo::ChannelMapAlg::SignalType
has two signatures, both virtual and subject to redefinition from derived classes.
When only one of them is overridden in a derived class, the other one is hidden.
This is in general not a concern since the derived classes are expected to be accessed only via the abstract interface, but it still is a design flaw.
A standard solution is to have the methods exposed as interface in geo::ChannelMapAlg
non-virtual, and have them call protected virtual implementation methods. These methods might have also different names to avoid the hiding problem. This solution is a breaking change to the implementation of the interface: all implementations which currently override any of the two methods will need to be fixed (the fix is trivial: move to protected and rename to the new name the overriding methods).
History
#1 Updated by Gianluca Petrillo about 3 years ago
- Subject changed from Fix design flaw in @geo::ChannelMapAlg@ interface to Fix design flaw in geo::ChannelMapAlg interface
#2 Updated by Lynn Garren about 3 years ago
- Status changed from New to Assigned
- Assignee set to Gianluca Petrillo
#3 Updated by Katherine Lato almost 3 years ago
- Assignee changed from Gianluca Petrillo to Paul Russo
#4 Updated by Paul Russo over 2 years ago
The SignalType()
virtual functions have been replaced with two non-virtual member functions:
geo::SigType_t SignalTypeForChannel(raw::ChannelID_t const channel) const;
geo::SigType_t SignalTypeForROPID(readout::ROPID const& ropid) const;
one for each of the original two overloads. Use these two functions in place of calling SignalType()
. Which one to use depends on the argument passed (in the obvious way).
A derived class implementer may customize the behavior of these functions by providing overrides for:
virtual geo::SigType_t SignalTypeForChannelImpl(raw::ChannelID_t const channel) const = 0;
virtual geo::SigType_t SignalTypeForROPIDImpl(readout::ROPID const& ropid) const;
which are called by the base class implementations of:
geo::SigType_t SignalTypeForChannel(raw::ChannelID_t const channel) const;
geo::SigType_t SignalTypeForROPID(readout::ROPID const& ropid) const;
Note that the implementer is required to provide an override for:
virtual geo::SigType_t SignalTypeForChannelImpl(raw::ChannelID_t const channel) const = 0;
as it is pure virtual.
#5 Updated by Paul Russo over 2 years ago
dunetpc, icaruscode, and lariatsoft were updated to adapt to this breaking change.
Released in v06_78_00.
#6 Updated by Paul Russo over 2 years ago
- Status changed from Assigned to Resolved
#7 Updated by Katherine Lato over 2 years ago
- Status changed from Resolved to Closed