Ticket #1884 (closed: fixed)
Geometry: Simplify the class hierarchy of ParametrizedComponent and other ParX classes.
Reported by: | Janik Zikovsky | Owned by: | Janik Zikovsky |
---|---|---|---|
Priority: | major | Milestone: | Iteration 26 |
Component: | Mantid | Keywords: | |
Cc: | pf9@… | Blocked By: | |
Blocking: | Tester: | Michael Whitty |
Description
This is a fairly large refactoring of the Geometry project, with the purpose of simplifying the class hierarchy significantly by folding in Parameterized versions of all the Components into a single class.
The design is currently this:
- a Component object holds the position etc. of an instrument's (base) component.
- For a parametrized instrument, a ParametrizedComponent is created which wraps the original Component with the addition of a const reference to a ParameterMap (m_map), as well as a pointer to the base (un-parametrized) Component (m_base).
- Every function in Component needs to have the same implemented in ParameterizedComponent.
- Most functions in ParameterizedComponent simply call m_base->function(...)
- Only a few require actually different behavior (e.g. getting position).
- Every function in Component needs to have the same implemented in ParameterizedComponent.
- The ParametrizedComponent is created by a factory.
This would not be so bad if there were only Component that needed parametrized version. However, EVERY class in the instrument needs its equivalent Par... version.
This THEN means that there needs to be an IComponent interface to interface between ParametrizedComponent and Component. So for each 1 REAL class, there are 2 MORE classes; triple the complexity!
This is then multiplied by all the classes that have their corresponding ParXXX and IXXX versions:
- Component
- Instrument
- ObjComponent
- CompAssembly
- ObjCompAssembly
- RectangularDetector
My proposed design:
- The Component object will be made to act as either a parametrized Component or a base component.
- The new Component can have a pointer to a ParameterMap object (m_map) containing its parameters, as well as a pointer to its base Component (m_base)
- If it DOES, that means it is a parametrized Component.
- Functions called on Component will check if it is parametrized, and act accordingly - either call the unparametrized m_base object, or perform the adjustments required by the ParameterMap.
- The ParComponentFactory still acts the same way - except it returns a new instance of Component with the ParameterMap and base set, instead of a different class.
Advantages:
- Two-to-Three-fold reduction in the required number of classes. Some of the interfaces will be kept at least at first; especially IComponent as it is used throughout the code.
- Maintenance of the code will be that much easier (modify one function instead of 2-3).
- I believe this will make the design much easier to understand for new developers.
Development:
- A separate branch will be created for this development, and kept merged with the running trunk to try to keep things up to date.
Change History
comment:2 Changed 10 years ago by Janik Zikovsky
To clarify my last point: development will be kept in the branch until complete, showing that it builds and tests well fully, before merging back to trunk when it is stable.
comment:10 Changed 10 years ago by Janik Zikovsky
comment:11 Changed 10 years ago by Janik Zikovsky
comment:12 Changed 10 years ago by Janik Zikovsky
comment:13 Changed 10 years ago by Janik Zikovsky
comment:14 Changed 10 years ago by Janik Zikovsky
comment:15 Changed 10 years ago by Janik Zikovsky
comment:16 Changed 10 years ago by Janik Zikovsky
comment:17 Changed 10 years ago by Janik Zikovsky
comment:18 Changed 10 years ago by Janik Zikovsky
comment:19 Changed 10 years ago by Janik Zikovsky
comment:20 Changed 10 years ago by Janik Zikovsky
comment:21 Changed 10 years ago by Janik Zikovsky
comment:22 Changed 10 years ago by Janik Zikovsky
comment:23 Changed 10 years ago by Janik Zikovsky
comment:24 Changed 10 years ago by Janik Zikovsky
comment:25 Changed 10 years ago by Janik Zikovsky
I meant, "The same tests fail for me in the trunk. " Also CurveFitting all tests from the same revision.
comment:26 Changed 10 years ago by Janik Zikovsky
comment:27 Changed 10 years ago by Janik Zikovsky
comment:28 Changed 10 years ago by Janik Zikovsky
comment:29 Changed 10 years ago by Janik Zikovsky
comment:30 Changed 10 years ago by Janik Zikovsky
comment:31 Changed 10 years ago by Janik Zikovsky
comment:32 Changed 10 years ago by Janik Zikovsky
comment:33 Changed 10 years ago by Janik Zikovsky
comment:34 Changed 10 years ago by Janik Zikovsky
comment:35 Changed 10 years ago by Janik Zikovsky
comment:36 Changed 10 years ago by Janik Zikovsky
comment:37 Changed 10 years ago by Janik Zikovsky
comment:38 Changed 10 years ago by Janik Zikovsky
comment:39 Changed 10 years ago by Janik Zikovsky
comment:40 Changed 10 years ago by Janik Zikovsky
comment:41 Changed 10 years ago by Janik Zikovsky
comment:42 Changed 10 years ago by Janik Zikovsky
comment:43 Changed 10 years ago by Janik Zikovsky
comment:44 Changed 10 years ago by Janik Zikovsky
comment:45 Changed 10 years ago by Janik Zikovsky
comment:46 Changed 10 years ago by Janik Zikovsky
comment:47 Changed 10 years ago by Janik Zikovsky
comment:48 Changed 10 years ago by Janik Zikovsky
- Status changed from accepted to verify
- Resolution set to fixed
comment:49 Changed 10 years ago by Janik Zikovsky
comment:50 Changed 10 years ago by Michael Whitty
- Status changed from verify to verifying
- Tester set to Michael Whitty
comment:51 Changed 10 years ago by Michael Whitty
- Status changed from verifying to closed
things are indeed much simpler in Geometry now. everything seems to have been working for the last five weeks of development so I'm going to go with the assumption that anything this might have broke would have been found by now.
comment:52 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 2731