2
\$\begingroup\$

I am writing simple file validator for my java ee app and I am stack with my class api. I need specific error descriptions, but also I would like to have boolean values indicating whether file is valid or not. Please give me some hints how to do it 'smart'.

package pl.poznan.put.ims.business.attachments;

import java.util.List;
import java.util.ArrayList;

import com.google.inject.Inject;

import pl.poznan.put.ims.business.entities.Attachment;
import pl.poznan.put.ims.business.exceptions.AttachmentValidationException;
import pl.poznan.put.ims.settings.IFileSystemSettings;

public class AttachmentValidator implements IAttachmentValidator 
{   
      IFileSystemSettings messageSettings;
      private List<String> errors;

    @Inject
    public AttachmentValidator(IFileSystemSettings messageSettings)
    {
        this.messageSettings = messageSettings;
        this.errors = new ArrayList<String>();
    }

    @Override
    public boolean validate(List<Attachment> attachments) throws AttachmentValidationException
    {
        for (Attachment attachment : attachments) {
            validate(attachment);       
        }

        if(attachments.size() > messageSettings.getMaxFilesCount())
            errors.add("Attachments max number exceeded.");

        if(!errors.isEmpty())
            throw new AttachmentValidationException(errors);

        return true;
    }

    @Override
    public boolean validate(Attachment attachment)
    {
        boolean isValid = validateLimitExceeded(attachment);
        isValid &= validateNull(attachment);

        return !isValid;
    }

    private boolean validateLimitExceeded(Attachment attachment)
    {           
        boolean exceeded = attachment.getFileSize() > messageSettings.getMaxFileSize(); 
        if(exceeded)
            errors.add("Max file size limit exceeded. File: " + attachment.getFileDisplayName() + ". Size: "+ attachment.getFileSize()/1024/1024 + "MB.");

        return exceeded;
    }

    private boolean validateNull(Attachment attachment)
    {
        if(attachment == null) {
            errors.add("No attachment.");   
            return false;
        }

        return true;
    }

    @Override
    public List<String> getErrors()
    {
        return errors;
    }

    @Override
    public long getTotalMaxFilesSize() {
        return messageSettings.getMaxFilesCount() * messageSettings.getMaxFileSize();
    }
}
\$\endgroup\$
2
\$\begingroup\$

Some notes:

  • The boolean validate(...) method never returns false so it should be void method.
  • I'd pass immutable ValidationResult objects to the clients. The clients could check the results and could signal to their clients if there is an error or prints the messages etc.
  • I'd omit the &= operator, it's really hard to read.
  • getTotalMaxFilesSize() should be in the FileSystemSettings class. (Feature or data envy smell.)

After a few refactoring steps the following came out:

AttachmentValidatorService.java

import java.util.List;

public interface AttachmentValidatorService {

    ValidationResult validate(List<Attachment> attachments);

    ValidationResult validate(Attachment attachment);

    long getTotalMaxFilesSize();
}

ValidationResult.java

import static com.google.common.base.Preconditions.checkNotNull;

import java.util.ArrayList;
import java.util.List;

import javax.annotation.concurrent.Immutable;

@Immutable
public class ValidationResult {

    private final List<String> errors;

    public ValidationResult(final List<String> errors) {
        checkNotNull(errors, "errors cannot be null");
        this.errors = new ArrayList<String>(errors);
    }

    public boolean isValid() {
        if (errors.isEmpty()) {
            return true;
        }
        return false;
    }

    public List<String> getErrors() {
        return new ArrayList<String>(errors);
    }

    // If lots of the clients use this
    //  public void checkErrors() {
    //      if (!isValid()) {
    //          throw new ...
    //      }
    //  }
}

AttachmentValidatorServiceImpl:

import static com.google.common.base.Preconditions.checkNotNull;

import java.util.Collections;
import java.util.List;

public class AttachmentValidatorServiceImpl implements
        AttachmentValidatorService {
    private final FileSystemSettings messageSettings;

    public AttachmentValidatorServiceImpl(
            final FileSystemSettings messageSettings) {
        this.messageSettings = checkNotNull(messageSettings,
                "messageSettings cannot be null");
    }

    @Override
    public ValidationResult validate(final List<Attachment> attachments) {
        final AttachmentValidator attachmentValidator = new AttachmentValidator(
                messageSettings, attachments);
        return attachmentValidator.getValidationResult();
    }

    @Override
    public ValidationResult validate(final Attachment attachment) {
        return validate(Collections.singletonList(attachment));
    }

    @Override
    public long getTotalMaxFilesSize() {
        return messageSettings.getMaxFilesCount()
                * messageSettings.getMaxFileSize();
    }
}

AttachmentValidator.java

import static com.google.common.base.Preconditions.checkNotNull;

import java.util.ArrayList;
import java.util.List;

public class AttachmentValidator {

    private final List<String> errors = new ArrayList<String>();

    private final FileSystemSettings messageSettings;

    public AttachmentValidator(
            final FileSystemSettings messageSettings,
            final List<Attachment> attachments) {
        this.messageSettings = checkNotNull(messageSettings,
                "messageSettings cannot be null");
        checkNotNull(attachments, "attachments cannot be null");
        for (final Attachment attachment : attachments) {
            validate(attachment);
        }

        validateSize(attachments);
    }

    private void validate(final Attachment attachment) {
        validateNull(attachment);
        validateLimitExceeded(attachment);
    }

    private void validateSize(final List<Attachment> attachments) {
        if (attachments.size() > messageSettings.getMaxFilesCount()) {
            errors.add("Attachments max number exceeded.");
        }
    }

    private void validateLimitExceeded(final Attachment attachment) {
        if (attachment == null) {
            return;
        }
        final int fileSize = attachment.getFileSize();
        final boolean exceeded = fileSize > messageSettings
                .getMaxFileSize();
        if (exceeded) {
            final String fileDisplayName = attachment
                    .getFileDisplayName();
            final int fileSizeInMegaBytes;
            if (fileSize == 0) {
                fileSizeInMegaBytes = 0;
            } else {
                fileSizeInMegaBytes = fileSize / 1024 / 1024;
            }
            final String errorMsg = "Max file size limit exceeded. File: "
                    + fileDisplayName
                    + ". Size: "
                    + fileSizeInMegaBytes + "MB.";
            errors.add(errorMsg);
        }
    }

    private void validateNull(final Attachment attachment) {
        if (attachment == null) {
            errors.add("No attachment.");
        }
    }

    public ValidationResult getValidationResult() {
        return new ValidationResult(errors);
    }

}

Feel free to ask if you have any questions.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.