task #8456: FieldUnitDTO: remove newInstanceMethod - EDIT - EDIT Project Management

Actions

Copy link

task #8456

closed

FieldUnitDTO: remove newInstanceMethod

Added by Andreas Kohlbecker over 3 years ago. Updated over 3 years ago.

Status:

Closed

Priority:

New

Assignee:

Katja Luther

Category:

cdmlib

Target version:

Release 5.10

Start date:

Due date:

% Done:

100%

Estimated time:

Severity:

normal

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 issues

Actions

Copy link

Updated by Andreas Kohlbecker over 3 years ago

Related to task #8457: FieldUnitDTO: remove taxonRelatedDerivedUnits added

Actions

Copy link

Updated by Andreas Kohlbecker over 3 years ago

Copied from task #8414: occurrence controller code cleaning and harmonization added

Actions

Copy link

Updated by Andreas Kohlbecker over 3 years ago

Related to deleted (task #8457: FieldUnitDTO: remove taxonRelatedDerivedUnits)

Actions

Copy link

Updated by Andreas Kohlbecker over 3 years ago

Related to task #8425: FieldUnitDTO: remove taxonRelatedDerivedUnits added

Actions

Copy link

Updated by Andreas Kohlbecker over 3 years ago

Tracker changed from bug to task

Actions

Copy link

Updated by Andreas Müller over 3 years ago

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"

Actions

Copy link

Updated by Katja Luther over 3 years ago

Status changed from New to Resolved
% Done changed from 0 to 50

Applied in changeset cdmlib|ed34f59fbc07b3029f7c6f939d2108fc324a7d08.

Actions

Copy link

Updated by Katja Luther over 3 years ago

Assignee changed from Katja Luther to Patrick Plitzner
% Done changed from 50 to 0

please review

Actions

Copy link

Updated by Patrick Plitzner over 3 years ago

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());
    }

Actions

Copy link

#10

Updated by Katja Luther over 3 years ago

Patrick Plitzner wrote:

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

Actions

Copy link

#11

Updated by Patrick Plitzner over 3 years ago

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.

Actions

Copy link

#12

Updated by Katja Luther over 3 years ago

Katja Luther wrote:

Patrick Plitzner wrote:

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());
    }