1

Well for the most of you this may be a simple question but I'm asking this to ensure that my class design isn't violating design concepts when it comes with proper use of constructor as well as access modifier.

In this class I'm working on I didn't use setters because I thought it would be correct to initialize Subject object by using the constructor instead of setting each field using setter methods. Hence, require my other teammates to supply the attributes of class Subject when it is instantiated before calling add() method.

My question is why I'm getting this warning on Netbeans?

enter image description here

  1. Should I make the instance variables private final?

  2. Should I just remove the constructor parameters and create setter methods to initialize the fields/variables?

Here's my code.

public class Subject {

    private String subjectName;
    private String subjectCode;
    private int subjectUnits;
    private String subjectDescription;
    private String subjectYearLevel;
    private int schoolYearStart;
    private int schoolYearEnd;

    public Subject(String name, String code, int units, String description, String yearLevel){
        subjectName = name;
        subjectCode = code;
        subjectUnits = units;
        subjectDescription = description;
        subjectYearLevel = yearLevel;
    }

    public void add(){

        String SQL = "INSERT INTO subject(name,code,units,description,yearlevel,creator) "
                + "VALUES (?,?,?,?,?,?)";
        try(Connection con = DBUtil.getConnection(DBType.MYSQL);
                PreparedStatement ps = con.prepareStatement(SQL);){
            ps.setString(1,subjectName );
            ps.setString(2,subjectCode );
            ps.setInt(3, subjectUnits);
            ps.setString(4, subjectDescription);
            ps.setString(5, subjectYearLevel);
            ps.setString(6, Login.getUsername());

        } catch (SQLException e) {
            JOptionPane.showMessageDialog(null,e.getClass()+" "+e.getMessage());
        }
    }
}

I'd appreciate any advice.

Thanks.

heisenberg
  • 1,784
  • 4
  • 33
  • 62
  • 5
    There are [some advantages](http://programmers.stackexchange.com/q/151733/108326) to making your `class` immutable, marking the fields final should do that. – Elliott Frisch May 25 '16 at 03:04
  • If there is no way of accessing to your variables other than that parameterized constructor then you should make your variables 'private final'. If you want access to your variables and do not want to use final keyword, I would suggest you to have the getters and setters. – Chit Khine May 25 '16 at 03:12
  • In this case, the fields are already effectively `final` (because you never change the value). Then you should make them `final`. This more clearly expresses your intent, and gives you some advantages, such as avoiding errors like this one: http://stackoverflow.com/q/37403097/14955 – Thilo May 25 '16 at 03:12

3 Answers3

1

Netbeans is giving that warning because you don't have any method that changes the original value of those fields, so Netbeans is recommending that in order to make your code more readable. Hence if you add any method that modifies that original value it will stop showing it.

About the design:

The decission between Inmutable and Mutable Objects it's about the purpose of those objects.

According to your example:

If your Subject class it's going to change in the Bussines Logic and those changes are going to have further use, in fact if the Subject holds an identity it's preferable to use a mutable class design, it doesn't need to be implemented with setters, it can also use a Factory.

Otherwise if your object doesn't have an identity and it is pro-performance to instantiate a new object every time you need a different one then use Inmutable, this also applies for concurrency, since inmutable objects are thread-safe.

As an example Hibernate, a Java ORM, need the models to be mutable and have setter methods.

If you want to read more about it:

6 Benefits of Programming with Immutable Objects in Java
If immutable objects are good, why do people keep creating mutable objects?

J. Pichardo

Community
  • 1
  • 1
J. Pichardo
  • 3,077
  • 21
  • 37
1

Since you have initialized your instance variables in constructor. And you have not reinitialized variable. Constructor is only going to call once in object life cycle. So netbeans suggesting you to declare these variables as final. If you use setter method then this warning will not be there. Because method can be called more than once, which is reinitialization of variables.

Akash
  • 74
  • 3
0

The warning is showed because you initialized the attributes using only constructor. And then never changed.

This warning will show at NetBean and will not show at Eclipse.

I think you can write your class like this :

public class Subject {

private String subjectName;
private String subjectCode;
private int subjectUnits;
private String subjectDescription;
private String subjectYearLevel;
private int schoolYearStart;
private int schoolYearEnd;

public Subject(String name, String code, int units, String description, String yearLevel){
    setName(name);
    setCode(code);
    setUnits(units);
    /*other setters*/
}

public String getSubjectYearLevel() {
     return subjectYearLevel;
}

public void setSubjectYearLevel(String subjectYearLevel) {
      /*Some validation and filters can write there*/
      this.subjectYearLevel = subjectYearLevel;
}

/*other getter setter.....*/


public void add(){

    String SQL = "INSERT INTO subject(name,code,units,description,yearlevel,creator) "
            + "VALUES (?,?,?,?,?,?)";
    try(Connection con = DBUtil.getConnection(DBType.MYSQL);
            PreparedStatement ps = con.prepareStatement(SQL);){
        ps.setString(1,subjectName );
        ps.setString(2,subjectCode );
        ps.setInt(3, subjectUnits);
        ps.setString(4, subjectDescription);
        ps.setString(5, subjectYearLevel);
        ps.setString(6, Login.getUsername());

    } catch (SQLException e) {
        JOptionPane.showMessageDialog(null,e.getClass()+" "+e.getMessage());
    }
}

}

In setter, you can write some validation codes of your attribute for someone doesn't know about your class.

Necromancer
  • 869
  • 2
  • 9
  • 25
  • 1
    There's absolutely nothing wrong with assigning values to fields in the constructor without setter methods, like in the original code. Your statement "You should access private attributes using only getter, setter." is plain false if you're accessing them from within the class. Your "solution" of *public* setter methods, just to get rid of the *suggestion* of the IDE, is unnecessarily breaking encapsulation and far worse than simply marking the fields `final`. – Arjan May 25 '16 at 05:15
  • 2
    Btw, you can put your validation in the constructor. I often use that pattern: immutable objects (`final` fields, no setters), constructor validates the parameters, assign if valid, throw Exception if invalid. That way, I end up with immutable objects of which I'm 100% sure they're valid if I have them anywhere in my code. It isn't always applicable but I'll use it whenever I can. It makes things so much easier - no `null` checks necessary, no duplicate code, simple handling of erroneous user input... – Arjan May 25 '16 at 05:32
  • @Arjan Thanks for your explanation – Necromancer May 25 '16 at 05:44
  • While there is a case that can be made about having setters here, unless there is a reason to enable those outside the context of this class, they should be made private. – Psycho Punch May 25 '16 at 07:14
  • @PsychoPunch Can you make the case? Personally I think there's absolutely no reason to use setters here, so I'm quite interested to hear. – Arjan May 25 '16 at 07:29
  • @Arjan I don't necessarily agree with it either, but if `/*Some validation and filters can write there*/` can be substantiated, then it can make acceptable point. – Psycho Punch May 25 '16 at 07:31