Reorganize class inheritance for `GenerateDownloadFile`

[fbastian]

In GitLab by @fbastian on Jul 14, 2015, 13:42

The classes in org.bgee.pipeline.expression.downloadfile have a problem of redundancy/inappropriate inheritances.

For instance, classes generating multi-species download files do not provide a list of species IDs to the constructor of GenerateDownloadFile, certainly it should.

The first task to fix this issue is to list all the problems related to these badly designed inheritances.

[fbastian]

In GitLab by @fbastian on Jul 14, 2015, 13:49

List of problems: (to be completed)

• org.bgee.pipeline.expression.downloadfile.GenerateMultiSpeciesDiffExprFile.DiffExpressionData is completely redundant with org.bgee.pipeline.expression.downloadfile.GenerateDiffExprFile.DiffExpressionData
• classes generating multi-species download files do not provide a list of species IDs to the constructor of GenerateDownloadFile
• the following methods are completely redundant: org.bgee.pipeline.expression.downloadfile.GenerateMultiSpeciesDiffExprFile.addDiffExprCallMergedDataToRow(MultiSpeciesCompleteDiffExprFileBean) and org.bgee.pipeline.expression.downloadfile.GenerateDiffExprFile.addDiffExprCallMergedDataToRow(SingleSpDiffExprFileType, Map, DiffExprCallType, DataState, DiffExprCallType, DataState)
• The method org.bgee.pipeline.expression.downloadfile.GenerateDiffExprFile.convertDataStateToString(DataState) is not used in the class GenerateMultiSpeciesDiffExprFile. As a result, GenerateMultiSpeciesDiffExprFile directly used the String value obtained from DataState, which gives results different from convertDataStateToString

[fbastian]

In GitLab by @vrechdelaval on Jul 15, 2015, 16:29

List of problems (next part):

• The method org.bgee.pipeline.expression.downloadfile.GenerateDiffExprFile.convertDataStateToString(DataState) is not used in org.bgee.pipeline.expression.downloadfile.GenerateExprFile. See 4th point of FB comment.
• With the actual bean hierarchy, when we implement org.bgee.pipeline.expression.downloadfile.GenerateExprFile.SingleSpeciesComplete/SimpleExprFileBean or org.bgee.pipeline.expression.downloadfile.GenerateDiffExprFile.SingleSpeciesComplete/SimpleDiffExprFileBean, some data columns (for instance, 'EST data' or 'Affymetrix best p-value' columns) will be defined twice because of org.bgee.pipeline.expression.downloadfile.GenerateMutliSpeciesExprFile.MutliSpeciesCompleteExprFileBean (not implemented yet) and org.bgee.pipeline.expression.downloadfile.GenerateMutliSpeciesDiffExprFile.MutliSpeciesCompleteDiffExprFileBean

[fbastian]

In GitLab by @fbastian on Jul 16, 2015, 13:15

Updated to priority high, because it doesn't make sense to address other issues using badly designed classes.

Updated to effort high because I have currently no idea how hard it is to refactor these classes :p

[fbastian]

In GitLab by @fbastian on Jul 16, 2015, 13:50

So, the general problem is that we would need double inheritances, which is not possible in Java: there would be a BaselineExpression/DiffExpression inheritance, and a SingleSpecies/MultiSpecies inheritance.

And we would like to have something like:

• GenerateExprFile extends BaselineExpression, SingleSpecies
• GenerateDiffExprFile extends DiffExpression, SingleSpecies
• GenerateMultiSpeciesExprFile extends BaselineExpression, MultiSpecies
• GenerateMultiSpeciesDiffExprFile extends DiffExpression, MultiSpecies

Actually, from the problems listed in this issue, it seems that only methods not using internal attributes of the classes are affected. So, it seems possible to implement all of them as default methods in interfaces, as we have moved to Java 8. And we could then use the pattern described above.

First, we need to list the default methods that should go in each of the four interfaces:

• ExpressionDownloadFile
• DiffExpressionDownloadFile
  • defines enum DiffExpressionData
  • provides default method addDiffExprCallMergedDataToRow
• MultiSpeciesDownloadFile
• SingleSpeciesDownloadFile

Parent abstract class of all classes generating download files:

• GenerateDownloadFile
  • convertDataStateToString: what is the reason for not having this method in this parent class? Isn't always the same for all types of download files?

[fbastian]

In GitLab by @fbastian on Jul 16, 2015, 14:13

About redundant fields in Beans, maybe there is nothing we can do. Note that a parent Bean could have fields not used by all the subclasses (and that it is not mandatory for a subclass to use all fields from its parent).

For instance, see org.bgee.pipeline.annotations.SimilarityAnnotation.CuratorAnnotationBean, extending RawAnnotationBean, but actually never using any of the 'label' fields defined in RawAnnotationBean and AnnotationBean.

This is even less of an issue, as we can define CellProcessors making sure that the fields not supposed to be used are never used.

[fbastian]

In GitLab by @fbastian on Oct 14, 2015, 23:09

Changed to priority medium, because I feel like the multi-specis present/absent expression files are going to use a completely different mechanism (based on bgee-core)