Accessing static methods of inner class from an outer class nonstatic constructor

Question

I have a public Unit class. I only want to access GeofenceUnit class THROUGH Unit class. Thus, I make GeofenceUnit an inner class of Unit. A Unit has many GeofenceUnits. Consequently, when I instantiate a Unit, I want to store many GeofenceUnits in the Unit object.

public class Unit {
  public ArrayList<Unit.GeofenceUnit> geofences;

  public Unit(int id){
    this.id = id;
    geofences = this.geofence.fill();

  }

  private static class GeofenceUnit {
    ArrayList<GeofenceUnit> geofences;
    private static ArrayList<Unit.GeofenceUnit> fill(){
      ...
      while(resultSet.next()){
        geofences.add(new Geofence());
      }

      return geofences;
    }
  }
}

The problem with the above code, as you may have noticed, is I am trying to call the static method fill() within the constructor of Unit. This yields the warning "The static method fill() from the type Unit.GeofenceUnit should be accessed in a static way". And I absolutely agree with the warning. I don't want to have to access it through a static way. However, if I remove the static modifiers from the GeofenceUnit class definition and its fill() method signature, then it doesn't logically make sense. Why would I populate many GeofenceUnits in an instance method. Good program practice suggest that method should be made static.

I think I just have a bad design right here. Any suggestions on how to refactor this?

Just a tip, if you're looking for suggestions for working code [CodeReview SE](https://codereview.stackexchange.com/) would be a nice place to post — awksp, Jun 12 '14 at 17:10
`if I remove the static modifiers from the GeofenceUnit class definition and its fill() method signature`. Make sure you understand what it means to have a `static` nested class. — Sotirios Delimanolis, Jun 12 '14 at 17:12
@JonSkeet I didn't know you could access the inner class that way. I thought you could only access inner class via this keyword. — JohnMerlino, Jun 12 '14 at 17:14
This presents other problems because I do need to access member variables of the outer class from the inner class and I cannot do that with the static inner class. — JohnMerlino, Jun 12 '14 at 17:22
@user3580294: see http://meta.stackoverflow.com/q/253975/217324. i don't think this is working code, so by that criteria this would be off-topic there. — Nathan Hughes, Jun 13 '14 at 01:06
@NathanHughes At least from how I read the question it's not that the code is broken (although if it is OP doesn't say so), but it seems that OP wants to refactor it and can't figure it out. So I *think* it would be Ok there. I definitely agree though that broken code shouldn't go there, but if I had thought that this were broken I wouldn't have recommended Code Review. — awksp, Jun 13 '14 at 01:53
@user3580294: i interpreted it as broken since it doesn't seem like it compiles in the state shown; this.geofence either doesn't exist or the code sample is incomplete (or there might be a typo), which is very possible. if this is incomplete it would help to see something more complete. and yes, for refactorings that makes sense. — Nathan Hughes, Jun 13 '14 at 02:05
@NathanHughes Now that you point it out, I also don't think that it would compile, but I'm more inclined to believe that it's a typo since the compiler at least got to the point where it could emit warnings instead of errors. — awksp, Jun 13 '14 at 02:51

score 1 · Answer 1 · answered Jun 12 '14 at 17:19

1

The 'this' modifier implies that the GeofenceUnit you are referring should be istantiated, which is impossibile since its static. You should access to the static method usong directly the class as Jhon Skeet says: GeofenceUnit.fill()

answered Jun 12 '14 at 17:19

Narmer

1,414
7
16

score 1 · Accepted Answer · edited May 23 '17 at 12:05

When you say in the constructor

geofences = this.geofence.fill();

that is saying that the class Unit has an instance member variable called geofence, which is not true here. (It's not part of the code sample shown, anyway.)

Also since the method you want to call is static no instance needs to be involved (and it is better style to avoid calling static methods on object instances). In order to call your static method fill() on GeofenceUnit you should change the line to

geofences = Unit.GeofenceUnit.fill();

If the method is unrelated to an instance of the class then it makes sense for the method to be static. But having a static method make a database call is ugly, it makes it hard to mock the results from the query, and entangles business logic with infrastructure code.

When you make an inner class static that means there is no connection between the outer class and it, the inner class does not have access to the outer class. So the static inner class can be instantiated independently of an outer class instance, but it doesn't have any special access to any instance of the outer class either. I'm not sure it makes sense to have this be a static inner class, or even an inner class.

I do not think you are making your life any easier by making JDBC calls in the constructor, or by putting the database access in a static method. The approach seems too concerned with privacy and not concerned enough with the single-reponsibility principle or easy unit testing.

An alternative would be to have domain objects that do not know how they are populated, with separate data access objects (otherwise known as repositories) to retrieve the data from the database and populate the domain objects.

Accessing static methods of inner class from an outer class nonstatic constructor

2 Answers2