CODEWITHSUNDEEP
javacoding-style
Mars
Mars

Reputation: 4307

private sub classes... is this a bad thing?

So I consider myself a junior java/android developer

I've always come across these but never really liked them and concidered them as dirty code

class herp{
 private class derp extends OnclickListener{
 ...
 }
 private class gerp AsyncTask{
 ...
 }
}

so should I try to avoid these? or even make sure I never use these?

Upvotes: 2

Views: 534

Answers (4)

BVSmallman
BVSmallman

Reputation: 601

What is and isn't clean code is often times personal preference based upon experience. Nesting classes isn't messes per say, however you should be certain that it is an appropriate situation.

If you desperately need some specific functionality for a class which extends something like the OnClickListener in your question, then it is a question of how many times do you need this class? If the answer is once, then an anonymous class would be a cleaner solution. If the answer is in every single method in the class, then a nested class clearly makes more sense.

More or less every feature in Java has a time and place in which it is considered appropriate. Private nested classes such as the ones you have in your question should be reserved in my mind for cases where you satisfy two conditions:

a) you absolutely have to have a separate class that will only be used in this class and no where else

AND

b) you will need to use that class in multiple locations within the class.

At the end of the day, nested private classes are not inherently dirty or hard to maintain, but as with any other feature of an programming language, make sure you need them.

Upvotes: 1

Bohemian
Bohemian

Reputation: 425378

It's fine. You may consider making them static inner classes, otherwise you'll need an instance of herp to create one (although that might be what you want):

class herp {
    private static class derp extends OnclickListener{
        ...
    }
    private static class gerp AsyncTask{
        ...
    }
}

The difference demonstrated is:

public static void main(String[] args) {
    // With static:
    new derp();
    // Without static:
    new herp().new derp();
}

Upvotes: 0

Laf
Laf

Reputation: 8215

There is no fixed answer on this question. It mainly comes down to your own coding style, preferences, and your team's coding conventions.

Private inner classes are useful for many reasons. You can use them to provide an implementation of an interface (e.g. a List implementation might define its own Iterator implementation as a private inner class) without making the concrete class visible. It protects the implementation, and allows you to provide just enough details to a user of your API/class so he can use it correctly, without cluttering your documentation with useless details (your concrete class).

You can also use private inner classes as a implementation for listeners, even though some might disagree with this philosophy. I do prefer using private inner classes to anonymous classes when the listener has some complex logic.

You might want to use them also to separate code logic into separate classes, but don't wish to expose those classes outsite your outer class.

Keep in mind that every solution using a private inner class can also be implemented without using them. And as with many things in life, using private inner classes isn't a bad practice per se, but abuse is a bad practice.

Upvotes: 0

corsiKa
corsiKa

Reputation: 82589

What is and isn't dirty code is highly subjective.

What can be said is that nested classes can be very useful. Often times they don't need to be nested like that, as they could just as easily be anonymous classes in all likelihood.

The idea is that: you want as few other classes as necessary to access your little class there. You wouldn't want to make your own package, because you really belong in the package you're already in. Instead, you make a private class. Now only you can use it, which is good because it's tailored just for your class.

Now, how many derp instances will you have? Chances are you'd have one. So instead of doing that, I would do this:

OnClickListener derp = new OnClickListener() {
    // fill in methods to override here
}

It does basically the same thing, but I can't reuse the class for anything, which is good - no one should be reusing the one-shot class!

Upvotes: 4

Related Questions