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

Intern AuditSystem in DB and getting users from it

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

Problem

My project has a change on the database, so that every INSERT-UPDATE-DELETE action is logged with triggers on the DB.

For that they have an extra table created:

Table T_USER is new and have a PK, a discriminator field and some other fields, Table T_APP_USER contains an PK witch is also FK to PK of T_USER.

In some Pojo's I need to show the last_changed_user, this should be normally comes from T_APP_USER but if a manuel intervention on the DB is done this could be an T_USER who is not known in T_APP_USER.

The following code works, but I'm interested in review, possibilities for other solution (user may be empty when it's a internal DB user). For the moment I add the AuditUser in the Pojo cause all the methods of User has been abstracted I can still get to all details of a User.

Abstract user class:

```
@Entity
@Table(name = "T_USER")
@Inheritance(strategy = InheritanceType.JOINED)
@DiscriminatorColumn(name = "USER_TYPE", discriminatorType = DiscriminatorType.CHAR)
public abstract class AuditUser extends PersistentEntity {

@Id
@GeneratedValue(generator = "T_USER_SEQ", strategy = GenerationType.SEQUENCE)
@SequenceGenerator(name = "T_USER_SEQ", sequenceName = "T_USER_SEQ", allocationSize = 1)
@Column(name = "user_sid", nullable = false)
private Integer id;
@Column(name = "STATUS")
private char status = 'A';

/**
* getter for id.
*
* @return the id of the user.
*/
@Override
public Integer getId() {
return this.id;
}

/**
* getter for status. A => Application user. D => Internal DB user.
*
* @return the status of the user
*/
public char getStatus() {
return status;
}

/**
* setter of the status(discriminator) A => Application user. D => Internal
* DB user.
*
* @param status the Status to set.
*/
public void setStatus(char status) {
this.status = status;
}

/**
* getter for account.

Solution

Your code looks good. I've visited this question multiple times and I couldn't find anything. Design-wise there might be improvements, but the implementation looks very good. The only thing I found was this.

/**
 * Equals written on account -ignoring the case and trimming-.
 *
 * @param obj the object to compare with
 * @return the boolean outcome
 */
@Override
public boolean equals(final Object obj) {
    if (this == obj) {
        return true;
    }
    if (obj == null) {
        return false;
    }
    if (this.getClass() != obj.getClass()) {
        return false;
    }
    final User other = (User) obj;
    if (this.account == null) {
        if (other.account != null) {
            return false;
        }
    } else if (!this.account.equals(other.account)) {
        return false;
    }
    return true;
}


This last bit:

if (this.account == null) {
        if (other.account != null) {
            return false;
        }
    } else if (!this.account.equals(other.account)) {
        return false;
    }
    return true;


Could be this instead:

if (this.account == null) {
        return other.account == null;
    } else {
        return this.account.equals(other.account);
    }


It simplifies things.

Additionally, I'd take a look at your comments, not all of them make sense...

/**
 * The is ldap account.
 */
@Column(name = "is_Ldap_Account", nullable = true)
private Boolean isLdapAccount = false;


And you seem to have misspelled your constants.

private static final int MAX_LENGHT_ACCOUNT = 30;
private static final int MAX_LENGHT_DEFAULT = 50;


(LENGHT -> LENGTH)

Code Snippets

/**
 * Equals written on account -ignoring the case and trimming-.
 *
 * @param obj the object to compare with
 * @return the boolean outcome
 */
@Override
public boolean equals(final Object obj) {
    if (this == obj) {
        return true;
    }
    if (obj == null) {
        return false;
    }
    if (this.getClass() != obj.getClass()) {
        return false;
    }
    final User other = (User) obj;
    if (this.account == null) {
        if (other.account != null) {
            return false;
        }
    } else if (!this.account.equals(other.account)) {
        return false;
    }
    return true;
}
if (this.account == null) {
        if (other.account != null) {
            return false;
        }
    } else if (!this.account.equals(other.account)) {
        return false;
    }
    return true;
if (this.account == null) {
        return other.account == null;
    } else {
        return this.account.equals(other.account);
    }
/**
 * The is ldap account.
 */
@Column(name = "is_Ldap_Account", nullable = true)
private Boolean isLdapAccount = false;
private static final int MAX_LENGHT_ACCOUNT = 30;
private static final int MAX_LENGHT_DEFAULT = 50;

Context

StackExchange Code Review Q#52577, answer score: 5

Revisions (0)

No revisions yet.