views:

192

answers:

11

Hello. I have created an anonymous class in which I declare a few variables and methods. My java teacher tells me to make these private. I don't see how changing the modifier makes any difference since these variables and methods are private to the anonymous class anyway, so I prefer to have no modifier at all. Who is right and what makes more sense? See below for example code where I choose no modifier for 'map' and 'convert' rather than making them private.

Collections.sort(list, new Comparator<String>(){
  public int compare(String a, String b){
    return convert(a).compareTo(convert(b));
  }
  Map<String, String> map = new HashMap<String, String>();
  String convert(String s) {
    String u = map.get(s);
    if (u == null)
      map.put(s, u = s.toUpperCase());
    return u;
  }
});
+1  A: 

Your professor is right. Make all class variable private and expose them via properties (if not anonymous).

The general rule of thumb is to keep member data such as variable including your Map object private.

JonH
Adding properties to an anonymous class seems of little use
Sander Rijken
@Sander for an anon class that is true I was referring to in general as a good rule of thumb.
JonH
As a general rule of thumb, add behaviour to the type - don't just drill holes into it.
Tom Hawtin - tackline
+5  A: 

I would be tempted to make them private simply for the fact that if you refactor the code and pull the anonymous class out as a standard class (Intellij, for example, can do this at the click of a button), having private fields is what you really want. You won't have to go and rework your classes to match your standard.

Brian Agnew
+3  A: 

Personally I would make them private (and final where possible) anyway - it's just a good habit to be in in general.

To put it another way: if you had to put an access modifier on (if, say, the keyword package was also used as an access modifier) what would you choose? Private, presumably - after all, you don't actually want to grant any other class access, do you?

Now, having decided that private is the most logically appropriate access modifier, I would make that explicit in the code.

Then again, I'd quite possibly not create an anonymous inner class with a member variable anyway - I'd be tempted to turn that into a named nested class instead.

Jon Skeet
+1  A: 

Default modifier is not the same as the private modifier, there're subtle differences. However, in your case it's more a religious question whether to make convert() default or private. I don't see any advantage in making it private though.

Anyway, your code has a memory leak as the String Cache is never cleared :-P Also, for even shorter/less code, use the Comparator String.CASE_INSENSITIVE_ORDER:

Collections.sort(list, String.CASE_INSENSITIVE_ORDER);
mhaller
Damn, forget that about the leak :-) I saw a static where there isn't any.
mhaller
Instances of the class are only used for one sort (over a finite collection). Therefore it is not a leak.
Tom Hawtin - tackline
+1  A: 

It really doesn't matter, but it's probably a good idea to keep your teacher happy as he/she will be grading you.

Jeremy Raymond
+1  A: 

I'd say it's a matter of style. You can't access the member map outside out of the anonymous class, but it might be best to define them as private for consistency with other classes.

If this were my code, I would say that if a class is complicated enough to need data members, it might be worth pulling it out into a separate class, in which case I'd certainly make the data members private.

Michael Williamson
+1  A: 

The key point is when you say "I don't see how changing the modifier makes any difference since these variables and methods are private to the anonymous class anyway"... you're assuming a lot about how your class is going to be used. Treat every class like it will be passed around and used in a variety of ways, in other words, use modifiers as appropriate. Besides, it makes the intent of class clear. It's not like Java is a terse language anyway, so you might as well be clear.

Tim Hoolihan
A: 

I don't see much benefit to marking things private just for the hell of it. It won't really gain you anything and someone reading the code might attach some significance to the choice when there really isn't any.

Dan
Can someone explain this? In this case, applying `private` just seems like cargo-cult programming. What difference does it actually make *in this specific case*?
Dan
I didn't downvote you and can't tell who has however I believe it is good practice to ensure member variables are excplicitly declared private. It is better to have some sort of accessability be defined then not. you never know when this anon class is no longer anon.
JonH
That may be. I guess I'm just not as much a fan of forced encapsulation as some others - just today I happened to run into a situation where I could have saved myself a lot of trouble had I declared a certain field as public instead of private. That probably affected my reply :)
Dan
+1 for learning from mistakes!
JonH
+1  A: 

You want these fields to be private, so mark them private.If a member is marked neither public not private then something suspicious is going on. Also mark fields that shouldn't change final. Keeping things standardised means less thinking, or at least less thinking on the irrelevant, and less to change when modifying code.

From a language point of view, the only real difference is that if you have extended a base class in the same package, you have now hidden fields or overridden "package-private" (default access) methods. The members can also be accessed via reflection (without setAccessible) by code in the same package (this can have mobile-code security implications).

Tom Hawtin - tackline
+1  A: 

I would question the need for all this complexity. Take a look at: String.compareToIgnoreCase()

David Soroko
A: 

difference between default and protected.

protected: object/method is accessible to all classes that are in the same package, and also accessible to sub/extension classes.

default: object/method is accessible to all classes that are in the same package.

What is your intention of your object/method and code modifier accordingly. Do not allow yourself to be confused when you come back to the code after six months because in huge projects you want to know that that object/method is or is not accessed anywhere else.

In three weeks, not just months, you would forget what the intended accessibility of those objects, 101% guaranteed. Then if you had a huge project and you had a hundred modifiers that were not specific and you desperately wanted to update the code, you would be frustrated by the compulsion to run reference check on those 100 objects/methods. May be someone took your jar and found the hidden cookies in them and used them, then you changed your code and broke someone's code.

Code your modifiers according to your intention unless you are either one or more of these:

  1. you have no further desire to work in large java projects.

  2. you are a extremely intelligent high functioning autistic person who has an indexed memory of every event of your life and can write a completely functional peer-peer file sharing service within two weeks on a lap top in a coffee shop.

  3. you deliberately use it as another tool to obfuscate your code.
Blessed Geek