Java best practice for outside access to inner private class fields

Question

I searched a lot for an answer to my question and found several options, would like to know what you this is best practice.

Use case: So I have a singleton class AccontManager, that has an inner class which is relevant to only it, which is a User class.

public class AccontManager {

private static final AccountManger ourInstance = new AccountManger();
private User user;

public static AccountManger getInstance()
{
    return ourInstance;
}

private AccountManger(){}

public User getUser(){
    return this.user;
}

private class User{
    private String id;
    
    private User (String id){
        this.id = id;
    }
}
}

Now, the situation is that User fields have to access to outside package class, but the user class is unique only to this singleton class, hence the inner class has to be private.

Optimally, creating public getter methods in the inner class to get the user fields would be best, but since the inner class is private, that's not possible.

Possible practice:

Practice: create respective getter methods in the outer AccountManager class for the User field.

Drawback: the user fields relate to the User, hence the outer class should not have getter methods to its fields.

Code example:

 public class AccountManger {

     private static final AccountManger ourInstance = new AccountManger();
     private User user;

     public static AccountManger getInstance()
     {
         return ourInstance;
     }

     private AccountManger(){}

     public User getUser(){
         return this.user;
     }

     public String getUserId() // <-- get User id
     {
         return user.id;
     }

     private class User{
         private String id;

         private User (String id){
             this.id = id;  
      }  
 }
 }

Practice: Change the User modifier to public, but keep the constructor private, so it couldn't be instantiated.

Drawback: the inner User class will be visible as a member of the AccountManager singleton class, which it shouldn't be.

Code example:

 public class AccountManager {

     private static final AccountManger ourInstance = new AccountManager();
     private User user;

     public static AccountManager getInstance()
     {
         return ourInstance;
     }

     private AccountManger(){}

     public User getUser(){
         return this.user;
     }

     public String getUserId() // <-- get User id
     {
         return user.id;
     }

     private class User{
         private String id;

         private User (String id){
             this.id = id;
         }
     }
     }

Practice:

Create a public interface, for example, IUser
Make inner class User implement that interface
Add to the outer class, in this case AccountManager a getter method to the User instance

Drawback: An interface needs to be used in order to obtain the data from User.

Code example:

    public class AccountManager {
    
    private static final AccountManager ourInstance = new AccountManager();
    private User user;

    public interface IUser{
        String getName();
    }

    private AccountManager(){}

    public static AccountManager getInstance(){
        return ourInstance;
    }

    public User getUser(){
        return this.user;
    }

    private class User implements IUser{
        private String id;

        private User(String id){
            this.id = id;
        }

        @Override
        public String getName() { // <-- get user id
            return id;
        }
    }
    }

So what do you think? Any one of the listed options? Some other way?

Thank you for your input

score 3 · Answer 1 · answered Jul 28 '17 at 14:30

I would say that the User class needs to be private, and that the AccountManager should implement the getters and setters to it's attributes.

You won't have to know what is a User, because the AccountManager will tell you what you can know. As a plus, you can obey the Law of Demeter, which is always good thing.

IMO, inner classes should be used as a way to make the code in the outer class cleaner, and they shouldn't be able to be used elsewhere.

But I guess making the constructor in User private is better than nothing.

score 0 · Answer 2 · answered Jul 28 '17 at 15:09

I see three alternative approaches:

Make the User class stand-alone and public (and give it an AccountManager field, if necessary). Then User becomes part of your public API - probably not what you want.
In the AccountManager class, create getters and setters for the publicly-relevant User properties. That keeps the User entity an invisible component of the Account Manager. (Analogy: if you talk about your car's horse powers, you are hiding the fact that it's a property not of the car itself, but of its engine, and that's quite OK).
If you want the notion of a user to become part of your API, but still want to keep your inner User class private, create an interface IUser with the publicly-relevant methods and have User implement IUser. Then you can expose a public IUser getUser() method in your AccountManager, and the outside world only sees the functionality declared in the interface.

score 0 · Answer 3 · answered Jul 28 '17 at 15:26

Seeing as AccountManager (you misspelled "Manager" in your code example) is a singleton class, it won't really make an effective difference, but you said yourself that "the user class is unique only to this singleton class", so the purpose of the User class might be better represented if you make it static.

Anyway, in your original code example, you included a method public User getUser(). With the class User being private, I don't really see a point in doing that, because outside the class AccountManager, any reference to a User object will be useless, because you will never be able to access any of its members or declare a User variable to which to assign the thus obtained User reference to. The only possible use case I could envision would be if AccountManager declared a public (or any access modifier higher than that of User) method that accepts a User parameter. But even if that were the case, I would question whether such a design is really what you want.

So you might want to think over whether you really need the method public User getUser(), provided User stays private. Which already leads us to your original question. I think your first proposed solution is better. This, to me, seems exactly the reason why a superclass is allowed to access `private' members of a subclass. Quoted from this answer on stackoverflow:

The inner class is (for purposes of access control) considered to be part of the containing class. This means full access to all privates.

So if you think of User as being part of AccountManager, then you don't have to feel guilty about accessing its private fields from within AccountManager but from outside User, because apparently, the designers of Java would also think that way.

Finally, if you don't intend to change the field id during the existence of a User, you could make id final.

Java best practice for outside access to inner private class fields

3 Answers3