2

I am reviewing the code of a colleague. In his entity object he has set nullable = false and he is also checking in the setter that the value to set is not null.

Is this useful? In any case, the nullable = false will throw an exception at some point. (The checkArgumentNotNull will throw an illegal argument exception if the value is null.)

private TypeChampMaterielDefaillant typeChamp;

@Column(name = "TYPE_CHAMP", nullable = false, length = 30)
@Enumerated(EnumType.STRING)
public TypeChampMaterielDefaillant getTypeChamp() {
    return typeChamp;
}

public void setTypeChamp(TypeChampMaterielDefaillant typeChamp) {
    checkArgumentNotNull(typeChamp, "typeChamp");
    this.typeChamp = typeChamp;
}

EDIT

So if I understand correctly nullable=false only apply to schema generation, thus if the database is not generated with the current entity it will be possible to persist a null value

Heetola
  • 4,431
  • 7
  • 26
  • 44

3 Answers3

3

Parameter nullable = false will happen on database operation (you won't be able so persist entity with this value equals to null). Additional check in setter is useful because you'll get exception earlier (during setter invocation) but not necessary.

Jakub Kubrynski
  • 12,678
  • 4
  • 56
  • 80
  • @PeterRader yes, but it will happen during persistence not invoking the setter – Jakub Kubrynski Aug 04 '14 at 14:41
  • Really, I have never seen it thrown an exception when you annotate nullable = false and save with null, I think that will come handy when you generate ddl schema with hibernate, hibernate might create constraint for it. – Elbek Aug 04 '14 at 14:41
  • @Elbek you're right, but if you'll use hibernate to generate schema it'll guarantee that there will be no null values. The idea of using setter is to get exception when setting value, not during persistence – Jakub Kubrynski Aug 04 '14 at 15:34
  • Correction, Hibernate does NOT guarantee, instead db server makes this happen. Hibernate just generates correct ddl, that is it. – Elbek Aug 04 '14 at 15:39
0

No, this is very bad practice. A Hibernate-Bean is a bean, any kind of intelligent setter is an workaround anyway.

Use the annotations instead like @NotNull!

Use asserations if you realy need something that is thrown in test/devel!

Grim
  • 4,939
  • 8
  • 46
  • 97
  • Any comment or "just because"? – Jakub Kubrynski Aug 04 '14 at 14:41
  • 1
    Shouldn't assertions be used only for debug/test purposes? – holap Aug 04 '14 at 14:42
  • @holap yes, you are right (i would say `debug/test/devel`). So use annotations instead. I only wrote it because the setter SHALL defunc if there is a null. If you need something that is thrown, then use a AsserationError instead, but never let it be part of the live-system. – Grim Aug 04 '14 at 14:44
0

I would say nullable = false is used for schema generation, not for validation from jpa(unless some third party library uses for validating before persist)

JPA 2.1 Specification:

11.2.2.1 Column 
The following elements of the Column annotation are used in schema generation: 
name 
unique 
nullable 
columnDefinition 
table 
length (string-valued columns only) 
precision (exact numeric (decimal/numeric) columns only) 
scale (exact numeric (decimal/numeric) columns only) 
See section 11.1.9 for the rules that apply to these elements and column creation. The AttributeOverride annotation may be used to override column
mappings.

As you can see jpa spec talks nothing about validation, unless hibernate/or some third party does. I am not positive either about hibernate validator for that nullable = false statement.

You better use some validator framework or use @PrePersist, @PreUpdate annotations.

Setter validation also bad, how about if user does not call it at all ?

Elbek
  • 3,314
  • 6
  • 35
  • 49
  • Hm, http://stackoverflow.com/questions/7439504 sais, its not for validation. My fault. – Grim Aug 04 '14 at 14:57