current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite

I am using JPA for my database work and I need to create a database entity superclass which all of my table entities will extend.

Every database entity will have a primary key named id (which in some cases is an int and in some a String) and also a lastUpdate field which stores the time at which the database row was last updated (set from within the code).

DatabaseEntity superclass:

import java.util.Date;
import javax.persistence.Basic;
import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.Id;
import javax.persistence.Temporal;
import javax.persistence.TemporalType;

@Entity
public class DatabaseEntity <T> {
    @Id
    @Basic(optional = false)
    @Column(name = "id")
    private T _id;

    @Basic(optional = false)
    @Temporal(TemporalType.TIMESTAMP)
    @Column(name = "last_update")
    private Date _lastUpdate;

    public T getId() {
        return _id;
    }

    public void setId(T id) {
        _id = id;
    }

    public Date getLastUpdate() {
        return _lastUpdate;
    }

    public void setLastUpdate() {
        _lastUpdate = new Date();
    }
}

Anything that I am doing wrong or that I should add in the superclass?

share|improve this question

2 Answers 2

up vote 2 down vote accepted

You should have something like this:

@MappedSuperclass 
public class DatabaseEntity <T extends Serializable> {

    @Id
    @Basic(optional = false)
    @GeneratedValue
    @Column(name = "id")
    private T id;

    public T getId() {
        return id;
    }

    @Version
    @Column(name = "VERSION")
    private Integer version;

    public Integer getVersion() {
        return version;
    } 

}

Links to interesting annotations

@MappedSuperclass

@Version

@GeneratedValue, @GeneratedValue

share|improve this answer
up vote 2 down vote

Looks fine for me, but such a class should be abstract, because it does not make sense to be able create instances of this type.

public abstract class DatabaseEntity <T>{
//
}

Making it abstract will reflect the actual abstraction of this entity.

One more thing, try to avoid java.util.Date as much as you can, it is just awful. You can use Joda time instead, or the new java date api if you using Java 8.

share|improve this answer
    
I will make it abstract and use the Java 8 date api, I have one question that came to mind though. Should I make the two fields protected and remove the setters and getters or should I leave it as it is? Could there be any implications in the future? –  Aki K Oct 4 '14 at 21:13
    
@SilliconTouch keep getters public and make the setters protected –  Sleiman Jneidi Oct 4 '14 at 21:19
    
+1 for discouraging Date @SilliconTouch If you can easily change it later, consider making everything as private as possible. This will remind you every time you access something you haven't needed yet. –  maaartinus Oct 5 '14 at 14:32

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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