Ticket #996 (closed: fixed)
Improve code structure in Algorithm
| Reported by: | Nick Draper | Owned by: | Janik Zikovsky |
|---|---|---|---|
| Priority: | major | Milestone: | Release 2.1 |
| Component: | Mantid | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | Tester: | Russell Taylor |
Description (last modified by Nick Draper) (diff)
There are several long methods in this class, consider if they can be refactored to make them easier to maintain.
- Execute - 175 line
- processGroups 107 lines
Also consider the following
Algorithm.h contains implementations of functions name() and version() from IAlgorithm.h that throw exceptions, with a comment that they must be overridden in any derived classes. Why are these implementations required, when a derived class would have to implement these functions anyway, since it would extend IAlgorithm?
Change History
comment:3 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:4 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:5 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:6 Changed 9 years ago by Nick Draper
- Milestone changed from Iteration 29 to Iteration 30
"New" tickets moved at the code freeze of iteration 29
comment:7 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:8 Changed 9 years ago by Nick Draper
- Milestone changed from Iteration 32 to Iteration 33
Moved to iteration 33 at iteration 32 code freeze
comment:9 Changed 9 years ago by Janik Zikovsky
- Owner changed from Russell Taylor to Janik Zikovsky
- Status changed from new to accepted
- Component set to Mantid
Accepting since I'm a masochist
comment:10 Changed 9 years ago by Janik Zikovsky
Refs #4554 Public ReadLock and private lock access
Also Refs #996: starting to clean up Algorithm.execute()
Changeset: be86fc7198eee110c3d262a25be9213da1b5de06
comment:11 Changed 9 years ago by Janik Zikovsky
Refs #996: unneeded catch blocks around processGroups
Changeset: bcecb4b77666c121a98984739928d47a5cb2785b
comment:12 Changed 9 years ago by Janik Zikovsky
Refs #4554 read/write locks on workspaces in Algorithm.
Also refs #996 cleanup
Changeset: ff60eaed87ed19b698f18594b4365b4b9c0568fc
comment:13 Changed 9 years ago by Janik Zikovsky
Refs #996: Re-organized Algorithm.cpp code
Changeset: 2f19428ebb7035c7a29c2500d1ca6674a1562b1c
comment:14 Changed 9 years ago by Janik Zikovsky
Refs #4647 #996: clean up a lot of group code.
4 tests fail at this point
Changeset: c9c9e0f4113e2a2b9b629df8e6614fe38f552d12
comment:15 Changed 9 years ago by Janik Zikovsky
Refs #996 clean up of Algorithm code.
Changeset: cce9d23068a299dabaf570f00f1743b0f1ecdeb4
comment:16 Changed 9 years ago by Janik Zikovsky
- Status changed from accepted to verify
- Resolution set to fixed
Cleaned up processGroups() a lot in #4647 and also Algorithm.cpp. execute() is still pretty long though because of the try/catch blocks.
comment:17 Changed 9 years ago by Russell Taylor
- Status changed from verify to verifying
- Tester set to Russell Taylor
comment:18 Changed 8 years ago by Russell Taylor
- Status changed from verifying to closed
Restricting the scope to the specific issues given in the description:
- name() and version() are now abstract methods.
- The long methods mentioned are still around the same length. However, processGroups() has a much better internal structure and, as Janik notes, much of execute() is devoted to catch blocks.
comment:19 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 1844
