dimanche 18 novembre 2018

OO design - is this design flawed?

I am creating a logic for web application to managing consents from user.

The model class that is persisted in the DB will have multiple fields, from which only a set will be changed with user request. E. g. class will have 10 fields with various consents, but user will be willing to change only 2 of those. To avoid writing a big chain of if-else's I designed this classes, to harness polymorphism to do the job for me, but somehow this design seems flawed to me. Could you tell me if this is proper way to do it?

PROBLEM: Change values of only subset of fields from large set of fields in class.

For sake of simplicity I removed getter/setters methods and some fields. Main logic for changing consents:

public class GdprServiceImpl implements GdprService {

private final ConsentRepository consentRepository;

@Autowired
public GdprServiceImpl(ConsentRepository consentRepository) {
    this.consentRepository = consentRepository;
}

@Override
public void changeConsent(User user, List<ConsentDto> consents) {
    Optional<Consent> optionalConsent = consentRepository.findByUser(user);
    if(optionalConsent.isPresent()) {
        Consent consent = optionalConsent.get();
        for(ConsentDto consentDto : consents) {
            consentDto.apply(consent);
        }
        consentRepository.save(consent);
    }
    else {
        Consent consent = new Consent();
        consent.setUser(user);
        for(ConsentDto consentDto : consents) {
            consentDto.apply(consent);
        }
        consentRepository.save(consent);
    }
}

Model class:

public class Consent {

    private Boolean messageConsent;
    private Boolean recordConsent;
    /*CONSTRUCTOR, OTHER METHODS AND FIELDS OMITTED*/
}

Classes that will change a set of fields from Consent class:

public abstract class ConsentDto {

    public abstract void apply(Consent consent);
}



public class RecordConsentDto extends ConsentDto {

   private boolean consentValue;

   public RecordConsentDto(boolean consentValue) {
        this.consentValue = consentValue;
    }

    @Override
    public void apply(Consent consent) {
        consent.setRecordConsent(consentValue);
    }
}



public class MessageConsentDto extends ConsentDto {

    private boolean consentValue;

    public MessageConsentDto(boolean consentValue) {
        this.consentValue = consentValue;
    }

    @Override
    public void apply(Consent consent) {
        consent.setMessageConsent(this.consentValue);
    }
}

Aucun commentaire:

Enregistrer un commentaire