Ticket #998 (closed: fixed)
Improve code Quality in Load instrument
| Reported by: | Nick Draper | Owned by: | Karl Palmen |
|---|---|---|---|
| Priority: | minor | Milestone: | Release 2.0 |
| Component: | Mantid | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | Tester: | Jean Bilheux |
Description
A number of large functions here (exec() is 243 lines!); consider refactoring as current code is very difficult to read and understand.
Change History
comment:3 Changed 11 years ago by Nick Draper
- Status changed from new to assigned
- Owner changed from Russell Taylor to Anders Markvardsen
Discuss code structure with Russell or Nick before starting
comment:4 Changed 10 years ago by Nick Draper
- Milestone changed from Iteration 26 to Iteration 27
Bulk move of tickets to iteration 27, if your ticket is essential for Iteration 26 then move it back.
comment:5 Changed 10 years ago by Nick Draper
- Milestone changed from Iteration 27 to Iteration 28
Bulk move of tickets at the end of iteration 27
comment:6 Changed 9 years ago by Nick Draper
- Milestone changed from Iteration 28 to Iteration 29
Bulk move of tickets at the end of iteration 28
comment:7 Changed 9 years ago by Nick Draper
- Milestone changed from Iteration 29 to Iteration 30
Accepted and assigned tickets moved at iteration 29 code freeze
comment:9 Changed 9 years ago by Russell Taylor
- Component set to Mantid
Some changes that fall under the remit of this ticket are going on under ticket #1944.
comment:10 Changed 9 years ago by Russell Taylor
Also to consider here: within its exec(), LoadParameter creates an instance of LoadInstrument so that it can call setComponentLinks. This has to be wrong. It suggests the method needs to be pulled out from being a member function of LoadInstrument, or that the logic should be somewhere different entirely.
comment:11 Changed 9 years ago by Nick Draper
- Milestone changed from Iteration 30 to Iteration 31
Bulk move of tickets to iteration 31 at the iteration 30 code freeze
comment:12 Changed 9 years ago by Anders Markvardsen
- Owner changed from Anders Markvardsen to Karl Palmen
Can u have look into Russell's comment above
comment:13 Changed 9 years ago by Karl Palmen
Actually, LoadParameter creates an instance of InstrumentDefinitionParser rather than LoadInstrument and InstrumentDefinitionParser has the setComponentLinks.
comment:14 Changed 9 years ago by Anders Markvardsen
Note also LoadInstrument was refactored in #3669, creating InstrumentDefintionParser and merging LoadInstrumentHelper into this file.
comment:16 Changed 9 years ago by Karl Palmen
- Status changed from accepted to verify
- Resolution set to fixed
Such changes have been made while dealing with query #3669.
comment:17 Changed 9 years ago by Jean Bilheux
- Status changed from verify to verifying
- Tester set to Jean Bilheux
comment:18 Changed 9 years ago by Jean Bilheux
- Status changed from verifying to closed
Look ok except just a typo in the documentation...but very minor.
comment:19 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 1846
(In [3374]) re #998 minor quality updates