task #8456
closed
FieldUnitDTO: remove newInstanceMethod
Added by Andreas Kohlbecker over 4 years ago.
Updated over 4 years ago.
Description
copied from #8414
FieldUnitDTO providing a newInstance method. This has to be removed:
/**
* @param fieldUnit
*/
public FieldUnitDTO(FieldUnit fieldUnit) {
super(fieldUnit);
}
public static FieldUnitDTO newInstance(FieldUnit fieldUnit){
FieldUnitDTO fieldUnitDto = new FieldUnitDTO(fieldUnit);
if (fieldUnit.getGatheringEvent() != null){
fieldUnitDto.gatheringEvent = GatheringEventDTO.newInstance(fieldUnit.getGatheringEvent());
}
fieldUnitDto.setRecordBase(fieldUnit.getRecordBasis().getMessage());
fieldUnitDto.setListLabel(fieldUnit.getTitleCache());
return fieldUnitDto;
}
is this by purpose? To me this looks like an error....
maybe this can be included into the constructor.
OccurrenceServiceImpl.assembleFieldUnitDTO(..) is using the constructor and adds the gathering event to the dto which is done in the newInstance method. --> TODO: needs review and harmonization (I think the newInstance Method should be removed completely as it causes confusion)
- Related to task #8457: FieldUnitDTO: remove taxonRelatedDerivedUnits added
- Copied from task #8414: occurrence controller code cleaning and harmonization added
- Related to deleted (task #8457: FieldUnitDTO: remove taxonRelatedDerivedUnits)
- Related to task #8425: FieldUnitDTO: remove taxonRelatedDerivedUnits added
- Tracker changed from bug to task
- Assignee changed from Andreas Müller to Katja Luther
- Target version changed from Unassigned CDM tickets to Release 5.10
Passing this ticket to Katja or Patrick as I never worked on/with this class.
Note: syntax of the method name is also not according to our standards. If it stays we should call it "NewInstance"
- Status changed from New to Resolved
- % Done changed from 0 to 50
- Assignee changed from Katja Luther to Patrick Plitzner
- % Done changed from 50 to 0
- Status changed from Resolved to Feedback
- Assignee changed from Patrick Plitzner to Katja Luther
- % Done changed from 0 to 90
looks fine. Only on question: Why did you remove the null check when creating the GatheringEvent -> https://dev.e-taxonomy.eu/redmine/projects/edit/repository/revisions/ed34f59fbc07b3029f7c6f939d2108fc324a7d08/diff
before:
if (fieldUnit.getGatheringEvent() != null){
fieldUnitDto.gatheringEvent = GatheringEventDTO.newInstance(fieldUnit.getGatheringEvent());
}
fieldUnitDto.setRecordBase(fieldUnit.getRecordBasis().getMessage());
fieldUnitDto.setListLabel(fieldUnit.getTitleCache());
after:
public FieldUnitDTO(FieldUnit fieldUnit) {
super(fieldUnit);
gatheringEvent = GatheringEventDTO.newInstance(fieldUnit.getGatheringEvent());
setRecordBase(fieldUnit.getRecordBasis().getMessage());
setListLabel(fieldUnit.getTitleCache());
}
Patrick Plitzner wrote:
looks fine. Only on question: Why did you remove the null check when creating the GatheringEvent -> https://dev.e-taxonomy.eu/redmine/projects/edit/repository/revisions/ed34f59fbc07b3029f7c6f939d2108fc324a7d08/diff
before:
if (fieldUnit.getGatheringEvent() != null){
fieldUnitDto.gatheringEvent = GatheringEventDTO.newInstance(fieldUnit.getGatheringEvent());
}
fieldUnitDto.setRecordBase(fieldUnit.getRecordBasis().getMessage());
fieldUnitDto.setListLabel(fieldUnit.getTitleCache());
after:
public FieldUnitDTO(FieldUnit fieldUnit) {
super(fieldUnit);
gatheringEvent = GatheringEventDTO.newInstance(fieldUnit.getGatheringEvent());
setRecordBase(fieldUnit.getRecordBasis().getMessage());
setListLabel(fieldUnit.getTitleCache());
}
the null check is now in the gatheringEventDto
Katja Luther wrote:
the null check is now in the gatheringEventDto
The parameter gathering is not checked in the newInstance() method.
public static GatheringEventDTO newInstance(GatheringEvent gathering){
GatheringEventDTO dto = new GatheringEventDTO();
gathering = HibernateProxyHelper.deproxy(gathering);
if (gathering.getLocality() != null){
That code woul fail if gathering was null.
Katja Luther wrote:
Patrick Plitzner wrote:
looks fine. Only on question: Why did you remove the null check when creating the GatheringEvent -> https://dev.e-taxonomy.eu/redmine/projects/edit/repository/revisions/ed34f59fbc07b3029f7c6f939d2108fc324a7d08/diff
before:
if (fieldUnit.getGatheringEvent() != null){
fieldUnitDto.gatheringEvent = GatheringEventDTO.newInstance(fieldUnit.getGatheringEvent());
}
fieldUnitDto.setRecordBase(fieldUnit.getRecordBasis().getMessage());
fieldUnitDto.setListLabel(fieldUnit.getTitleCache());
after:
public FieldUnitDTO(FieldUnit fieldUnit) {
super(fieldUnit);
gatheringEvent = GatheringEventDTO.newInstance(fieldUnit.getGatheringEvent());
setRecordBase(fieldUnit.getRecordBasis().getMessage());
setListLabel(fieldUnit.getTitleCache());
}
the null check is now in the gatheringEventDto
do you have the actual code??
in my repository it looks like this:
public FieldUnitDTO(FieldUnit fieldUnit) {
super(fieldUnit);
if (fieldUnit.getGatheringEvent() != null){
gatheringEvent = GatheringEventDTO.newInstance(fieldUnit.getGatheringEvent());
}
setRecordBase(fieldUnit.getRecordBasis().getMessage());
setListLabel(fieldUnit.getTitleCache());
}
sorry I missed to add the last commit to this ticket.
- Status changed from Feedback to Resolved
- Assignee changed from Katja Luther to Patrick Plitzner
- Status changed from Resolved to Closed
- Assignee changed from Patrick Plitzner to Katja Luther
- % Done changed from 90 to 100
Also available in: Atom
PDF