Ticket #3692 (closed: fixed)
ParameterMap name tags are all over the code
Reported by: | Martyn Gigg | Owned by: | Karl Palmen |
---|---|---|---|
Priority: | major | Milestone: | Release 2.5 |
Component: | Mantid | Keywords: | |
Cc: | anders.markvardsen@… | Blocked By: | |
Blocking: | Tester: | Gesner Passos |
Description (last modified by Nick Draper) (diff)
The ParameterMap contains parameter names such as pos,rot,masked that are used all over the codebase. This needs centralizing so that the string representation is only in one place.
There is also no unit test for ParameterMap on its own. It is tested in conjunction with the Par* tests at the moment.
Change History
comment:2 Changed 9 years ago by Nick Draper
- Owner set to Anyone
- Status changed from new to assigned
- Description modified (diff)
- Summary changed from ParameterMap name tags are all of the code to ParameterMap name tags are all over the code
comment:3 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:4 Changed 8 years ago by Nick Draper
- Milestone changed from Release 2.1 to Release 2.2
Moved at end of release 2.1
comment:5 Changed 8 years ago by Nick Draper
- Milestone changed from Release 2.2 to Release 2.3
Moved at the end of release 2.2
comment:6 Changed 8 years ago by Nick Draper
- Milestone changed from Release 2.3 to Release 2.4
Moved to milestone 2.4
comment:10 Changed 8 years ago by Karl Palmen
I've identified the following string that are used in the parameter map:
"pos", "rot" "x", "y", "z" "rotx", "roty", "rotz" "double", "int", "bool", "string", "V3D", "Quat"
Also there are some mores strings used in asString and SaveNexus. I could write a function in the header file for each one of them to return it.
comment:11 Changed 8 years ago by Martyn Gigg
My thought here is rather than just free functions they should be static functions on the ParameterMap class itself, i.e for the "pos" parameter Header:
static const std::string & pos();
Source:
namespace // to be above all of the other function defs { const std:string POS_PARAM_NAME="pos"; //... plus the others } const std::string & ParameterMap::pos() { return POS_PARAM_NAME; } // ... plus the others
The POS_PARAM_NAME part is important as it stops the string from being constructed over and over again.
comment:12 Changed 8 years ago by Karl Palmen
Thank you Martyn for the suggestion.
comment:14 Changed 8 years ago by Karl Palmen
Made the changes to the code re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: c579eb77200f8e6b70f97467a68d511e0780a1e1
comment:15 Changed 8 years ago by Karl Palmen
Provisional plan for unit test
Test 1: Test constructor works. Verify parameter map is empty and has zero size.
Test 2: Construct and add one parameter. Verify parameter map is not empty and possibly check its size.
Test 3: Check all the parameter names given by the parameter name functions are correct. This will be the only test that explicitly uses parameter names. The other tests will use the functions tested here.
Test 4: Test the add... functions verify with the contains function. Also test size function, if not done in test 2.
Test 5: Test the get... functions.
Test 6: Test the SaveNexus does not throw any exceptions.
comment:16 Changed 8 years ago by Karl Palmen
I find there is already a ParameterMapTest.h . It follows a different plan initially similar to mine.
Test 3: needs adding and could be named testParameter_Name_Functions(). Also it should be the only one to make explicit use of the parameter names. Other functions would call the functions tested here.
Testing ParameterMap.empty() function could be added to testAdding_A_Parameter_That_Is_Not_Present_Puts_The_Parameter_In().
Possibly add testSaveNexus_Does_Not_Throw .
comment:17 Changed 8 years ago by Karl Palmen
Remove explicit references to parameter names in unit test re #3692
These will be confined to the test testParameter_Name_Functions() to be added.
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: d33ae58835513e4dd3431bf4d0aa8c561ce5bd6b
comment:18 Changed 8 years ago by Karl Palmen
Remove more explicit names from unit test re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 8409e31f09c6d03020ca3d1289b17e79cd984292
comment:19 Changed 8 years ago by Karl Palmen
Add test for the Parameter Name Functions re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 089e71c9b2f8fa409351b4ba0623b2414c195d23
comment:20 Changed 8 years ago by Karl Palmen
Made the changes to the code re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: c579eb77200f8e6b70f97467a68d511e0780a1e1
comment:21 Changed 8 years ago by Karl Palmen
Remove explicit references to parameter names in unit test re #3692
These will be confined to the test testParameter_Name_Functions() to be added.
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: d33ae58835513e4dd3431bf4d0aa8c561ce5bd6b
comment:22 Changed 8 years ago by Karl Palmen
Remove more explicit names from unit test re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 8409e31f09c6d03020ca3d1289b17e79cd984292
comment:23 Changed 8 years ago by Karl Palmen
Add test for the Parameter Name Functions re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 089e71c9b2f8fa409351b4ba0623b2414c195d23
comment:24 Changed 8 years ago by Karl Palmen
Test empty function re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: e8c0ee2d23e4d60896e09061fd85a624b0553323
comment:25 Changed 8 years ago by Karl Palmen
Test empty function re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: e8c0ee2d23e4d60896e09061fd85a624b0553323
comment:26 Changed 8 years ago by Karl Palmen
To test, check the code of ParameterMap.cpp to see that all strings inserted into the parameters are defined in one place.
Check that the unit test ParameterMapTest.h exists and tests the functions that return these strings and that the unit test does not explicitly refer to these strings anywhere else.
Check that the unit test tests the empty function.
You may also check that the unit test passes.
comment:27 Changed 8 years ago by Karl Palmen
- Status changed from accepted to verify
- Resolution set to fixed
comment:28 Changed 8 years ago by Karl Palmen
Made the changes to the code re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: c579eb77200f8e6b70f97467a68d511e0780a1e1
comment:29 Changed 8 years ago by Karl Palmen
Remove explicit references to parameter names in unit test re #3692
These will be confined to the test testParameter_Name_Functions() to be added.
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: d33ae58835513e4dd3431bf4d0aa8c561ce5bd6b
comment:30 Changed 8 years ago by Karl Palmen
Remove more explicit names from unit test re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 8409e31f09c6d03020ca3d1289b17e79cd984292
comment:31 Changed 8 years ago by Karl Palmen
Add test for the Parameter Name Functions re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 089e71c9b2f8fa409351b4ba0623b2414c195d23
comment:32 Changed 8 years ago by Karl Palmen
Test empty function re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: e8c0ee2d23e4d60896e09061fd85a624b0553323
comment:33 Changed 7 years ago by Gesner Passos
- Status changed from verify to verifying
- Tester set to Gesner Passos
comment:34 Changed 7 years ago by Karl Palmen
Made the changes to the code re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: c579eb77200f8e6b70f97467a68d511e0780a1e1
comment:35 Changed 7 years ago by Karl Palmen
Remove explicit references to parameter names in unit test re #3692
These will be confined to the test testParameter_Name_Functions() to be added.
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: d33ae58835513e4dd3431bf4d0aa8c561ce5bd6b
comment:36 Changed 7 years ago by Karl Palmen
Remove more explicit names from unit test re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 8409e31f09c6d03020ca3d1289b17e79cd984292
comment:37 Changed 7 years ago by Karl Palmen
Add test for the Parameter Name Functions re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 089e71c9b2f8fa409351b4ba0623b2414c195d23
comment:38 Changed 7 years ago by Karl Palmen
Test empty function re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: e8c0ee2d23e4d60896e09061fd85a624b0553323
comment:39 Changed 7 years ago by Gesner Passos
- Status changed from verifying to closed
Internally, the problem is solved. But, outside the ParameterMap, there are plenty of references to the direct names of the parameters. (For example, Code/Mantid/Framework/Crystal/src/SCDCalibratePanels.cpp uses the names and not the indirect function ::x() ::y() ::pos()) My question is if we should open a ticket to review the places where those parameters appears and change to the newly created functions.
comment:40 Changed 7 years ago by Karl Palmen
Made the changes to the code re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: c579eb77200f8e6b70f97467a68d511e0780a1e1
comment:41 Changed 7 years ago by Karl Palmen
Remove explicit references to parameter names in unit test re #3692
These will be confined to the test testParameter_Name_Functions() to be added.
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: d33ae58835513e4dd3431bf4d0aa8c561ce5bd6b
comment:42 Changed 7 years ago by Karl Palmen
Remove more explicit names from unit test re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 8409e31f09c6d03020ca3d1289b17e79cd984292
comment:43 Changed 7 years ago by Karl Palmen
Add test for the Parameter Name Functions re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 089e71c9b2f8fa409351b4ba0623b2414c195d23
comment:44 Changed 7 years ago by Karl Palmen
Test empty function re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: e8c0ee2d23e4d60896e09061fd85a624b0553323
comment:45 Changed 7 years ago by Karl Palmen
Made the changes to the code re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: c579eb77200f8e6b70f97467a68d511e0780a1e1
comment:46 Changed 7 years ago by Karl Palmen
Remove explicit references to parameter names in unit test re #3692
These will be confined to the test testParameter_Name_Functions() to be added.
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: d33ae58835513e4dd3431bf4d0aa8c561ce5bd6b
comment:47 Changed 7 years ago by Karl Palmen
Remove more explicit names from unit test re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 8409e31f09c6d03020ca3d1289b17e79cd984292
comment:48 Changed 7 years ago by Karl Palmen
Add test for the Parameter Name Functions re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 089e71c9b2f8fa409351b4ba0623b2414c195d23
comment:49 Changed 7 years ago by Karl Palmen
Test empty function re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: e8c0ee2d23e4d60896e09061fd85a624b0553323
comment:50 Changed 7 years ago by Karl Palmen
Made the changes to the code re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: c579eb77200f8e6b70f97467a68d511e0780a1e1
comment:51 Changed 7 years ago by Karl Palmen
Remove explicit references to parameter names in unit test re #3692
These will be confined to the test testParameter_Name_Functions() to be added.
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: d33ae58835513e4dd3431bf4d0aa8c561ce5bd6b
comment:52 Changed 7 years ago by Karl Palmen
Remove more explicit names from unit test re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 8409e31f09c6d03020ca3d1289b17e79cd984292
comment:53 Changed 7 years ago by Karl Palmen
Add test for the Parameter Name Functions re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 089e71c9b2f8fa409351b4ba0623b2414c195d23
comment:54 Changed 7 years ago by Karl Palmen
Test empty function re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: e8c0ee2d23e4d60896e09061fd85a624b0553323
comment:55 Changed 7 years ago by Karl Palmen
Made the changes to the code re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: c579eb77200f8e6b70f97467a68d511e0780a1e1
comment:56 Changed 7 years ago by Karl Palmen
Remove explicit references to parameter names in unit test re #3692
These will be confined to the test testParameter_Name_Functions() to be added.
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: d33ae58835513e4dd3431bf4d0aa8c561ce5bd6b
comment:57 Changed 7 years ago by Karl Palmen
Remove more explicit names from unit test re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 8409e31f09c6d03020ca3d1289b17e79cd984292
comment:58 Changed 7 years ago by Karl Palmen
Add test for the Parameter Name Functions re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 089e71c9b2f8fa409351b4ba0623b2414c195d23
comment:59 Changed 7 years ago by Karl Palmen
Test empty function re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: e8c0ee2d23e4d60896e09061fd85a624b0553323
comment:60 Changed 7 years ago by Karl Palmen
Made the changes to the code re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: c579eb77200f8e6b70f97467a68d511e0780a1e1
comment:61 Changed 7 years ago by Karl Palmen
Remove explicit references to parameter names in unit test re #3692
These will be confined to the test testParameter_Name_Functions() to be added.
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: d33ae58835513e4dd3431bf4d0aa8c561ce5bd6b
comment:62 Changed 7 years ago by Karl Palmen
Remove more explicit names from unit test re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 8409e31f09c6d03020ca3d1289b17e79cd984292
comment:63 Changed 7 years ago by Karl Palmen
Add test for the Parameter Name Functions re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: 089e71c9b2f8fa409351b4ba0623b2414c195d23
comment:64 Changed 7 years ago by Karl Palmen
Test empty function re #3692
Signed-off-by: Karl Palmen <karl.palmen@…>
Changeset: e8c0ee2d23e4d60896e09061fd85a624b0553323
comment:65 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 4539
Bulk move of tickets to iteration 31 at the iteration 30 code freeze