12

I' have a question about initialization of List in the POJO as it follows the next code:

public class Person {

 //other fields...
 private List<String> friends=new ArrayList<>();

     public List<String> getFriends() {
        return friends;
     }
     public void setFriends(List<String> friends) {
        this.friends = friends;
    }

}

OR is it better like this and have initalization in other class(like for example Bean(JSF))

public class Person {

 //other fields...
 private List<String> friends;

     public List<String> getFriends() {
        return friends;
     }
     public void setFriends(List<String> friends) {
        this.friends = friends;
    }

}

So my question is what approach is better?

Tiny
  • 27,221
  • 105
  • 339
  • 599
Mitja Rogl
  • 894
  • 9
  • 22
  • 39
  • 7
    If they call getter first, you're screwed. – Sotirios Delimanolis Apr 02 '13 at 20:07
  • Is it a `managedBean`? – Eng.Fouad Apr 02 '13 at 20:09
  • @Eng.Fouad Yes it is managedBean. – Mitja Rogl Apr 02 '13 at 20:13
  • @KevinCrowell, do not initialize in the constructor within the context of JSF. It has implications – kolossus Apr 02 '13 at 21:15
  • 1
    @kolossus We didn't know this was a managed bean before. Given the new info, I agree, so deleted my comment. – Kevin Crowell Apr 02 '13 at 21:22
  • Almost every people who answered this question thought `Person` as a normal Java class. But, since it's a JSF managed bean, the best suited answer would be @kolossus'. – Luiggi Mendoza Apr 02 '13 at 22:05
  • @LuiggiMendoza OP failed to indicate this in his question clearly enough. – kolossus Apr 02 '13 at 22:10
  • @kolossus I read that from comments. **For future readers**, you should initialize your data in the `@PostConstruct` method (usually named `init`) as posted and explained by kolossus. – Luiggi Mendoza Apr 02 '13 at 22:22
  • 1
    In reality, you do not need to rely on `@PostConstruct` ALL the time. You only need that annotation when and only when you need to wait for some dependency injection to be done so that you can use some services to initialize the list. If not, what you're doing in your question is perfectly fine :). However, I do have a bad feeling that your current architecture is not the best practice. Do you have any entities called `Person` in the database? – Mr.J4mes Apr 03 '13 at 09:11
  • @Mr.J4mes Currently I don't have database in this app. It is just for testing. So tell me why is that bad practise, if I have entities called Person? – Mitja Rogl Apr 03 '13 at 22:12
  • something called `Person` should be an entity that is injected into a `@ManagedBean`. It should never be a `@ManagedBean` by itself. For example, it should be a property like `private Person person` injected in a bean called `MrBean`. Then you can access the property `friends` of your `Person` entity in your page using `mrBean.person.friends`. `MrBean` will contain all the functions required for your `Person` such as `addFriend()`, `removeFriend()`, `blockFriend()`, etc. – Mr.J4mes Apr 04 '13 at 03:14

5 Answers5

26

If it's a managed bean as you say, you should do this in a method annotated with @PostConstruct

public class Person {
    private List<String> friends;
    @PostConstruct
    public void init(){
         friends = new ArrayList<String>();
    }

    //getter and setter...
}
  1. The practice of doing any initialization in the getter and setter is generally frowned upon within the context of JSF. See Why JSF calls getters multiple times

  2. Also, per the API for @PostConstruct, the contract specifies safety features and guarantees that if an exception is thrown in a method annotated as such, the bean should not be put into service. There are no such guarantees on a plain constructor.

  3. In a managed bean, injection happens immediately after construction. This means that any operations you're carrying out in the constructor cannot depend on any injected resources (via @ManagedProperty). Whereas in a @PostConstruct method, you'll have access to all the resources declared on the managed bean

EDIT: It's important to note that there can be only one @PostConstruct for any @ManagedBean, so all important initializations should happen in there.

It's also worthwhile to note that, while the @PostConstruct method is the ideal place to initialize a backing bean variable/List, there are implications regarding the scope of the managed bean

  1. @RequestScoped: In a managed bean with this annotation, the method will be called per submit of the JSF view concerned. A @RequestScoped bean is destroyed and recreated with every request, The implication of this is that depending on your setup, the list initialized in the @PostConstruct may be reset to empty or default values during each request. Under certain circumstances, conversion errors may occur as a result of the re-initialization of the list mid-JSF request.

  2. @ViewScoped: In a managed bean with this annotation, you're guaranteed to have the @PostConstruct method run once, if and only if you're dealing with the same instance of the @ViewScoped bean. If the viewscoped bean is destroyed and recreated, the @PostConstruct method will run again.

  3. @SessionScoped: A bean with this annotation is created once and stays alive until the user's HTTP session ends. In this scenario, the @PostConstruct method is guaranteed to run once and only once until the bean is destroyed

See also

Community
  • 1
  • 1
kolossus
  • 20,559
  • 3
  • 52
  • 104
  • Please no one should edit the link I referenced. I need it for a reference on meta – kolossus Apr 02 '13 at 21:03
  • It would be worth to add this in your answer (directly from the `@PostConstruct` documentation): *Only one method can be annotated with this annotation.* – Luiggi Mendoza Apr 02 '13 at 22:25
  • 1
    Very good take on JSF side, as usual, but in this case I see absolutely no reason to use `@PostConstruct` over constructor, or initializer blocks. In the first place I'd recommend to use post construct method preferrably only for the sake of using initialize dependencies that have relevant annotations. I would recommend to initialize bean properties independent of injected dependencies in constructor, or in initializer block, as they naturally belong there. – skuntsel Apr 03 '13 at 05:18
4

I would suggest this:

public class Person {
     //other fields...
     private List<String> friends=new ArrayList<>();

     // returns a copy to protect original list
     public List<String> getFriends() {
        Collections.unmodifiableList(new ArrayList<>(friends));
     }
     public void addFriend(String> friend) {
        this.friends.add(friend);
     }
     public void addFriends(List<String> friends) {
        this.friends.addAll(friends);
     }
}
anubhava
  • 761,203
  • 64
  • 569
  • 643
  • You probably want to `clear` or do a defensive copy in the `set` otherwise it isn't really a setter more an `addAll`. – Boris the Spider Apr 02 '13 at 20:12
  • @bmorris591: Thanks I changed the method name to avoid confusion. – anubhava Apr 02 '13 at 20:16
  • 1
    Very similar to my own answer, but perhaps better design overall to have the add and addAll methods like this. I suspect we were typing at the same time. +1 – cobaltduck Apr 02 '13 at 20:23
3

In my opinion it would be best to handle that in the constructors. If a default constructor is used, initialize the list in the constructor.

public Person() {
    friends = new ArrayList<>();
}

If a constructor which accepts parameters is used, let the calling class pass in a list.

public Person(ArrayList<> friends) {
    this.friends = friends;//friends
}
kolossus
  • 20,559
  • 3
  • 52
  • 104
Haz
  • 2,539
  • 1
  • 18
  • 20
  • Within a managed bean, it would not be best to handle this in the constructor – kolossus Apr 02 '13 at 20:52
  • Yes, my example works better for POJO rather than a managed bean. Although I don't think there's any harm in initializing in the default no-arg constructor with managed beans if you're not using managed properties. – Haz Apr 02 '13 at 21:18
  • Since this is a basic example, it will work. However, as posted in section 3 of kolossus answer, none of the injected fields (like `@EJB`, `@ManagedProperty`, etc.) on the managed bean will work on the class constructor, instead in the `@PostConstruct` method. – Luiggi Mendoza Apr 02 '13 at 22:27
2

My suggestion, add a null check in the getter:

public class Person {
  //other fields...
  private List<String> friends;

  public List<String> getFriends() {
     if (this.friends == null) friends = new ArrayList<String>();
     return friends;
  }
}

But also notice I have omitted the setter. Instead, in any client code, call like this:

personInstance.getFriends().add("Some Item");

Or if you have a full list to add:

personInstance.getFriends().addAll(someStringCollection);
cobaltduck
  • 1,598
  • 5
  • 25
  • 51
  • Depends what the OP wants the behaviour to be. This is a good strategy known as "lazy initialization". You will also need a no-arg constructor if this is to be a managed bean. – Richard Wеrеzaк Apr 02 '13 at 20:19
1

It depends. Usually first way preferable because you may want to add something to collection later. If you won't know was your collection initialized or not you must check it every time.

Maxim Kolesnikov
  • 5,075
  • 6
  • 38
  • 68