Java return method correct when populating an object
Sorry for the ambiguous title. I'll edit if anyone has a better way to explain this.
Let's say I have a class in which I create many instances and which has many attributes that are filled in in different ways. Is it bad practice to pass an object and fill in attributes in methods? I'll try to explain with an example I tried to keep it simple:
public class User {
private String surname;
private String name;
public String getSurname() {
return surname;
}
public void setSurname( String surname ) {
this.surname = surname;
}
public String getName() {
return name;
}
public void setName( String name ) {
this.name = name;
}
}
// Passing the object as parameter and returning the object in each methods
// In this case getNameFromSomewhere returns a User object
public User getUser(){ //edit: my mistake here
User user= new User();
user = getNameFromSomewhere(user);
user = getSurnameFromSomewhere(user);
return user;
}
In my case, getNameFromSomewhere does a search on the server, I was wondering if I should change all of my methods so that it returns a string exactly like the attribute, then just:
// Alternative ?
public User getUser(){ //edit: my mistake here
User user= new User();
user.setName(getNameFromSomewhere()); // getNameFromSomewhere return string
user.setName(getSurnameFromSomewhere());
return user;
}
* note: I have string, int, list attributes to fill. Edit: I wrote an alternative, I'm just wondering how smart it is to pass data to the user and then return it with 1 attribute per attribute, or if I just have to use the User.set method to populate the attribute and my methods return the attribute type. (is this a little clearer?)
source to share
The problem with your code is that the class User
exposes its internal functionality through setter methods, violating the principle of information hiding . Your approach may lead to an unreachable code base i.e. It will be difficult to keep track of all the components that can change the object User
.
I think the best approach is to have a constructor that takes directly the information needed to build User
.
public class User {
private String surname;
private String name;
public User(String name, String surname) {
this.name = name;
this.surname = surname;
}
public String getSurname() {
return surname;
}
public String getName() {
return name;
}
}
Then you can create a user like this:
public User getUser() {
User user = new User(getNameFromSomewhere(),
getSurnameFromSomewhere());
return user;
}
This way you are sure where your user came from and that it cannot be changed anywhere. Moreover, this code follows the principle of single responsibility , because methods getNameFromSomewhere
and getSurnameFromSomewhere
are solely responsible for obtaining the first / last name.
The optimal approach should be one that uses an immutable class implementation User
. This means that every time you need to change an object, you create a copy of it with the desired information changed. Thus, the whole testing process becomes easier.
source to share
yes, it's better to have a getNameFromSomewhere()
return value instead of accepting a custom object and calling the setter. There are two reasons for this:
- You want to decouple the interface that the custom class provides. so when you refactor the last name into the last name you don't need to change
getNameFromSomewhere()
- perhaps it
getNameFromSomewhere()
will be used to populate an attribute of some other bean besides Account.
source to share