which of these ways to verify object is better in perspective of readable and maintainable?

Question

I'm programming Human Resource data sync batch program.

This program reads user and department data from customer company's database and save to our database.

For example, there is class named 'User' indicates each user.

It has 4 member variables, userId, userName, departmentCode, userRank.

In our database, there is 'user' table has columns user_id, user_nbame, department_code, user_rank.

Each member variable save each column like below example.

• userId -> user_id(primary key)
• userName -> user_name(not null)
• departmentCode -> department_code(not null)
• userRank -> user_rank(not null)

Before I save user data to our database, I need to verify user data.

Because user table's user_rank column has not null constraints, so it may cause ConstraintException.

In order to avoid ConstaintException, I thought I need to use isValid method.

This 'isValid' method checks whether member variables are null or empty string.

If at least one of member variables is null or empty string, return false.

If All member variables are not null or not empty string, return true.

Because user_id is primary key, so must not be null and duplicated, user_name must not be null, department_code must not be null, and userRank must not be null.

user_id, user_name, department_code columns don't have problem.

Because these 3 columns obviously essential in all companies.

But, user_rank column was null in some customer company.

So I use 'hasLength' for this 'isValid' method.

Below 2 methods have same purpose. These just verify itself and return true or false.

import org.springframework.util.StringUtils;

public class User {
    private String userId;
    private String userName;
    private String departmentCode;
    private String userRank;

    // first way
    public boolean isValid() {
        return  StringUtils.hasLength(this.userId) &&
                        StringUtils.hasLength(this.userName) &&
                        StringUtils.hasLength(this.departmentCode) &&
                        StringUtils.hasLength(this.userRank);
    }
}

import org.springframework.util.Assert;

public class User {
    private String userId;
    private String userName;
    private String departmentCode;
    private String userRank;

    // second way
    public boolean isValid() {
        try {
            Assert.hasLength(this.userId);
            Assert.hasLength(this.userName);
            Assert.hasLength(this.departmentCode);
            Assert.hasLength(this.userRank);

            return true;
        } catch(IllegalArgumentException e) {
            return false;
        }
    }

At first, I used first way.

But, some team members said "it isn't easily readable. "

After hearing that opinion, I referenced some spring framework codes and improved by using 'Assert' class my code like second way.

Do you think what is better way?

Or if you think other better way than i thought, can you tell me something?

---

---

I apologize about my mistake.

I wrote question so ambiguous, so I reinforced question.

If you satisfied by this editing, please reopen this question.

Thank you.

Answer 1

The second method is more readable, but there are some parts of your code you have to revisit:

1. hasLength is less readable than isEmpty
2. You should not catch your exception directly in this method. Instead call a throw new statement. It will allows another classes using this method, to catch and manage the exception and send a custom message.

A sample code:

public boolean isValid() {
    if (this.userId.isEmpty() || this.departmentCode.isEmpty() || this.departmentCode.isEmpty()) {
        throw new IllegalArgumentException("YOUR CUSTOM MESSAGE")
    }
    return true;
}

Update:

• You can use bean validation, to directly apply your constrains on your fields with annotations.