Scaffold Hunter / Bug reports / #300 DbManagerHibernate.getAccPropertyMinMax(): Some parameters not used for scaffold properties

Till Schäfer - 2018-03-27

Blocks: --> FR 10
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Till Schäfer - 2018-03-27

please fix this issues in a complete manner first, you can then rely on a solid implementation in FR 10

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Description has changed:

Diff:

--- old
+++ new
@@ -1,12 +1,13 @@
 The method getAccPropertyMinMax() in DbManager has seven arguments:
-* tree
-* propDef
-* acc
-* subset
-* subtreeCumulative
-* removeVirtualRoot
-* includeMoleculeData
+ tree
+ propDef
+ acc
+ subset
+ subtreeCumulative
+ removeVirtualRoot
+includeMoleculeData
+

 If *propDef* is a scaffold property and *subtreeCumulative* is set to false, *removeVirtualRoot* and *acc*  are not used by the implementation in DbManagerHibernate. *subset* is allowed to be null in this case, which would lead to incorrect results or a crash in other cases. Also *includeMoleculeData* is not used for scaffold properties which is however mentioned in the method's documentation.

-In feature #10 some changes are applied to this method, so this ticket is blocked by this feature.
+In feature #10 this method is used and modified, so it is blocked by this ticket.

Philipp Mewes - 2018-03-29

status: open --> in-progress
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Philipp Mewes - 2018-04-03

assigned_to: Philipp Mewes
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

The following table shows the results of further analysis. Empty cells mean that the parameter is not used in this case.

tree	propDef	acc	subset	sC	rVR	iMD
tree	scafProp		subset	false
tree	scafProp	Acc	subset	true	{true, false}
tree	molProp	Sum	subset	{true, false}	{true, false}	{true, false}
tree	molProp	Acc \ {Sum}	subset			{true, false}

Last edit: Philipp Mewes 2018-04-04

Description has changed:

Diff:

--- old
+++ new
@@ -8,6 +8,6 @@
 includeMoleculeData


-If *propDef* is a scaffold property and *subtreeCumulative* is set to false, *removeVirtualRoot* and *acc*  are not used by the implementation in DbManagerHibernate. *subset* is allowed to be null in this case, which would lead to incorrect results or a crash in other cases. Also *includeMoleculeData* is not used for scaffold properties which is however mentioned in the method's documentation.
+If *propDef* is a scaffold property and *subtreeCumulative* is set to false, *removeVirtualRoot* and *acc*  are not used by the implementation in DbManagerHibernate. There are similiar combinations where some parameters are not used depending on the value of other parameters.  Perhaps *includeMoleculeData* is not used for scaffold properties which is however mentioned in the method's documentation. It is also not allowed to set the subset to null and perform the requestedoperation on the whole dataset, which makes it impossible to use this method when no subset is present.

 In feature #10 this method is used and modified, so it is blocked by this ticket.

Philipp Mewes - 2018-04-09

Added the new method getScaffoldPropertyMinMax() to the DatabaseManager for the following reasons ([c53d9f]):
If the minimum/maximum of a scaffold property is required the respective call is getAccPropertyMinMax(tree, scafProp, null, false, false, false). In this case three parameters are not used, especially the parameter acc for the AccumulationFunction(see feature #10). This is a bit misleading, since the method that calculates an accumulated value does not apply any accumulation in this case.
The new method is called by getAccPropertyMinMax() anyway to maintain compatibilty with other calls.

Related

Commit: [c53d9f]

Last edit: Philipp Mewes 2018-04-25

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Philipp Mewes - 2018-04-13

Please have a look, if this solution is sufficient. I also tried an approach with splitting the method into two variants, one for molecules and one for scaffolds, but i wasn't content with that.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Philipp Mewes - 2018-04-13

status: in-progress --> needs-review
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Philipp Mewes - 2018-05-25

status: needs-review --> in-progress
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Philipp Mewes - 2018-05-25

As discussed in the meeting i removed the new methods([e5d05f]) and updated the javadoc comment ([b782e1]).

Related

Commit: [b782e1]
Commit: [e5d05f]

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Philipp Mewes - 2018-05-25

status: in-progress --> needs-review
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

DbManagerHibernate.getAccPropertyMinMax(): Some parameters not used for...

Group

Searches

Help

#300 DbManagerHibernate.getAccPropertyMinMax(): Some parameters not used for scaffold properties

Discussion

Related

Related