HiveBrain v1.2.0
Get Started
← Back to all entries
debugjavaMinor

Error-handling / Self-Validating mechanism

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handlingerrorvalidatingmechanismself

Problem

The below idea seemed to look clean, to allow the object itself to validate its values for different scenarios.

For eg:

  • While creating the object, the value of Object1 and Object2 in SelfValidator object should be validated for not null.



  • At one scenario like inserting the SelfValidator object to Database, validation of value1 as non-zero has to be done, which is not necessary for an Update. A new validate function: validateForInsert(), which will validate and set the error data, can be included to SelfValidator class.



But I am sure, there might be some drawbacks, design flaws, which I would like to understand and improve.

ErrorInfo.java

public class ErrorInfo {
  String errorMessage = "";

  public void setErrorMessage(String errorMessage) {
    this.errorMessage = new StringBuffer(this.errorMessage).append("\n+").append(errorMessage).toString();
  }
}


SelfValidator.java

public class SelfValidator {

  String object1;
  String object2;
  int value1;
  ErrorInfo errorInfo;

  public ErrorInfo getErrorInfo() {
    return errorInfo;
  }

  public SelfValidator(String object1, String object2, int value1) {
    super();
    this.object1 = (object1 == "")?null: object1;
    validate(this.object1, "object1");
    this.object2 = (object2 == "")?null: object2;
    validate(this.object2, "object2");
    this.value1 = value1;
  }

  private void validate(String value, String dataMember) {
    if(value == null){
        if(errorInfo == null){
            errorInfo = new ErrorInfo();
        }
        errorInfo.setErrorMessage(dataMember +" value is invalid");
    }
  }
}


Demonstrator.java

```
public class Demostrator {
public static void printValidOrNot(ErrorInfo errorInfo){
if(errorInfo == null){
System.out.println("Object valid");
//Proceed with manipulating the object
}else{
System.out.println(errorInfo.errorMessage);
// Break and handle the error.
}
}
public static void main(String[] args)

Solution

isValid Method

If you have a validator, I would expect a method to check if it is valid.

If you add an isValid method, you would get rid of the errorInfo == null check, which isn't a good idea. It exposes how the validator works internally, and relies on this never changing.

errorMessage Type

Why is your error message a String? You are creating a new StringBuffer and calling toString every time an error is appended. Just make errorMessage a StringBuffer or StringBuilder and call toString when the error message is retrieved (which should be done via a getter, not direct access).

Naming

setErrorMessage: What this actually does is appendErrorMessage

Spacing

I know it's knit-picking, but please follow the general conventions regarding spacing. Before and after ?, :, and + should be a space.

General Idea

I'm not too sure what this is good for (maybe I'm missing something?). It doesn't seem like the SelfValidator class can be reused for any other data. The three fields and their types are hardcoded, as is the validation mechanism (no empty string and no null allowed). Couldn't all your code be replaced with something like this:

private boolean isValid = true;

  public MyClass(String object1, String object2, int value1) {
    super();
    if (object1 == null || object2 == null || object1.equals("") || object2.equals("")) {
        this.isValid = false;
    }
  }

  public isValid() {
    return isValid;
  }


It seems just as flexible as your approach, but is easier to read.

Code Snippets

private boolean isValid = true;

  public MyClass(String object1, String object2, int value1) {
    super();
    if (object1 == null || object2 == null || object1.equals("") || object2.equals("")) {
        this.isValid = false;
    }
  }

  public isValid() {
    return isValid;
  }

Context

StackExchange Code Review Q#61933, answer score: 2

Revisions (0)

No revisions yet.