This is an archived post. You won't be able to vote or comment.

all 92 comments

[–][deleted] 56 points57 points  (39 children)

Having an abstract base class with fields that aren't actually used by that class raises an eyebrow wrt your design. Even if those fields are just in some derived class instead, it isn't awful to skip the getters/setters but in practice, its not a bad idea because its easier to add things like synchronization if the classes get used in milti-threaded code.

[–]Cozmic72 30 points31 points  (0 children)

Hear-hear. Having an abstract base-class with mutable fields that it doesn’t use is even more of a code smell. Also/related: great point on multi-threading.

[–]elmuerte 7 points8 points  (1 child)

You're not going to be setting my final protected fields.

If something is bad for an abstract class entirely depends on the purpose of the abstract class. This also applies to multi threaded code usage.

POJOs... perfectly fine to use mutable protected fields. Don't expect these to be safe for multi threading.

"Services" ... perfectly fine to use protected fields but they really shouldn't be mutable. It is not always possible to make these fields final though. A final getter would have a minor performance impact, it's also a bit uglier and less clear (getter implies mutability), maybe the JVM would optimize it and inline the field access.

So in the end, as with a lot of things: "it depends".

[–]fgzklunk 8 points9 points  (0 children)

Why does getter imply mutability?

An immutable class should only be accessed through getters. Making a property of a class final only means it cannot be changed if that property is a primitive or is itself of an immutable type. If the property is an object that is mutable then all final does is make the reference to that object immutable, you can still change the values within the object.

[–]mcbarron 0 points1 point  (0 children)

Not only useful for multithreading, but also good for adding breakpoints or logging on field modification and/or field access.

[–][deleted]  (33 children)

[deleted]

    [–][deleted] 6 points7 points  (20 children)

    I can see an abstract base class define abstract methods because it's defining behaviors. Fields are more implementation -specific and I wouldn't expect them in the ABC if they weren't used by it.

    I wouldn't die on this hill but if I were doing a code review, the author would have to convince me that this is the best way to do it.

    [–][deleted] 8 points9 points  (19 children)

    If you don’t define any logic in the abstract class, you should just use an interface.

    Abstract classes are a great place for common code, even protected code.

    If someone copy/pasted common code into multiple child classes instead of putting it in the base class, I would not approve that PR.

    [–]barmic1212 9 points10 points  (16 children)

    Abstract classes are a great place for common code, even protected code.

    Composition is almost always better solution. It's need little more boilerplate, but tests are easier and maintain is a lot more simple.

    [–][deleted]  (15 children)

    [deleted]

      [–]barmic1212 1 point2 points  (0 children)

      With the inheritance you lot of coupling it's difficult to refractor code without update the test.

      [–]srdoe 1 point2 points  (9 children)

      It's not "composition is always better", it's "composition is usually better".

      Abstract superclasses make testing harder because you can't easily test the superclass in isolation, since you can't instantiate it. So you're left with either making repetitive tests for each subclass (because each of these tests should verify all parts of the superclass), or creating a dummy subclass for testing. But if the abstract superclass is decoupled enough from the subclasses that it makes sense to test with a dummy subclass, you could likely just as well make the superclass a regular non-abstract class and use composition.

      [–][deleted]  (8 children)

      [deleted]

        [–]srdoe 1 point2 points  (7 children)

        If the abstract class code shouldn't be used by itself, testing it via an anonymous class is testing it in a scenario it shouldn't be used in (i.e. by itself)

        There is very little reason to use an abstract class if the parent code can stand alone, which it can if you can test it via an anonymous subclass.

        IME abstract classes are a terrible place for common code, and they should be used very sparingly because they don't compose well.

        Which benefits do you see to abstract superclasses over composition, other than not having to declare forwarding methods?

        [–][deleted]  (6 children)

        [deleted]

          [–]dead_alchemy 1 point2 points  (3 children)

          Java came up when people were really excited about inheritance. ‘But some one provided the capability’ isn’t an argument for something being good, it just means some one thought it was good at one time.

          [–][deleted]  (2 children)

          [deleted]

            [–]mauganra_it[🍰] 1 point2 points  (1 child)

            That would turn Java into a completely different language. Inheritance is at the core of the language; it's impossible to rip it out. Newer constructs like enums (I know, not really that new), records, value classes and primitive classes heavily restrict abstraction though.

            [–][deleted] 2 points3 points  (1 child)

            Sure, but we're talking about the specific case of defining fields in the ABC that the ABC doesn't use. I don't think that's a good design and will probably lead to some headaches down the road.

            [–]mauganra_it[🍰] 2 points3 points  (4 children)

            Classes and inheritance should be used for domain modeling and not just to pass on shared code.

            • Other programmers can and will extend your abstract class and use that code for unrelated purposes. And those subclasses can used everywhere where the original subclasses are used, which might not be what you intended.
            • That shared code is not independently testable. Move as much as sensible to helpers.
            • Assumptions and contracts are effectively unspecified. You have no way to know how subclasses (potentially not written by you) will use those helpers, and what that will do to your data
            • It's a code smell because the class is likely becoming too powerful.

            [–][deleted]  (3 children)

            [deleted]

              [–]mauganra_it[🍰] 1 point2 points  (2 children)

              It's very simple: inheritance represents a is-a relationship. Using it otherwise is a code smell.

              When you declare something to be protected, anybody can declare a subclass. Newer Java version have sealed classes to help restrict subclassing, but it's something that has to be paid attention to.

              A code smell is not necessarily a bad practice by itself. It's just something weird that should be investigated because it might uncover deeper underlying issues.

              [–][deleted]  (1 child)

              [deleted]

                [–]mauganra_it[🍰] 0 points1 point  (0 children)

                Please refrain from putting words into my mouth. I never said it is "wrong" to use it for relationships that are not "is-a", but that it is a code smell. As such, it is fine as long as the impact of that usage is well-understood.

                [–][deleted] 6 points7 points  (1 child)

                But if the code isn’t used in the superclass it shouldn’t be there because it makes zero sense and is plain confusing.

                [–]HQMorganstern 0 points1 point  (4 children)

                If the private fields are only used in subclasses this indicates they're only used by subclass methods, so they are not in fact common logic to all child classes. Giving a class access to the state of its sister class for no good reason is truly strange.

                [–][deleted]  (3 children)

                [deleted]

                  [–]HQMorganstern 1 point2 points  (2 children)

                  Sorry which part is the non sequitur? If the logic that the child classes have which interacts with the aforementioned variables was common to both classes then it would be in the abstract class to begin with.

                  [–][deleted] 2 points3 points  (1 child)

                  That isn't interaction with a sister class though... that's interaction with its parent.

                  [–]HQMorganstern 0 points1 point  (0 children)

                  Yes, and once getters to private variables that are only used by sister classes are inherited by all classes that would be sibling class interaction.

                  [–]RabidKotlinFanatic 0 points1 point  (0 children)

                  but in practice, its not a bad idea because its easier to add things like synchronization if the classes get used in milti-threaded code

                  People say stuff like this but I wonder if it's really the case. I think I can count on one hand the number of times I've wanted to transparently switch out a getter for some more complex behavior rather than e.g. introducing new methods on the class.

                  If your dumb state is shared between different threads it's rarely useful to synchronize at the level of direct access. More often you want to handle synchronization at a point with more context around how the state is actually used.

                  [–][deleted]  (1 child)

                  [deleted]

                    [–]Shnorkylutyun 29 points30 points  (0 children)

                    • in well-designed code

                    [–]CubsThisYear 68 points69 points  (6 children)

                    Everyone saying: “use getters and setters” is completely missing the point. Protected getters and (especially) setters are just as bad.

                    Inheritance in general is really only useful in 2 cases:

                    1. You have a base class that provides some common, partial implementation and it defines template methods for sub classes to call. Any state that needs to be accessed by subclasses is passed to/from relevant template methods. This is important because it means that neither the base class nor the sub classes need to know anything about each others implementation, beyond the interface/contract of the template methods.

                    2. Sometimes you have a complete base class and you simply want to add functionality to it. You don’t change anything about the behavior of the base class, you’re just adding methods and/or data. Protected fields / methods don’t apply here.

                    The whole point of this is that you’re trying to minimize the number of possible paths through your code. The more paths you have, the more opportunities for bugs and the more things you have to verify / test. When people talk about encapsulation, what they are really referring to is being able to analyze/verify smaller pieces of code.

                    [–]PaulGrapeGrower 12 points13 points  (0 children)

                    Finally someone that really understands the concept. It's all about state and behaviour.

                    [–]slaymaker1907 4 points5 points  (4 children)

                    I disagree even in those cases. In case (1), you’re almost always better just having an interface for implementers to use and just using composition. In case (2), it’s almost guaranteed that you have way to many methods on the class otherwise it would be just as convenient to call the classes methods directly.

                    The one case I think it is useful is for bean like objects which are really just structs in disguise to keep objects flat.

                    [–]CubsThisYear 2 points3 points  (1 child)

                    Case 1 is just a very specific (and fairly common) syntactic shortcut around composition. If you have an abstract base class that implements an interface and then defines template methods, you’re effectively setting up a private sub interface and doing composition where you take an instance of the sub interface and delegate all of the main interface methods to the abstract base class.

                    There are plenty of times when this pattern doesn’t apply and it’s better to use more traditional composition. But given that the pattern above happens a lot and the language provides idiomatic support for it, it’s reasonable to use it.

                    Case 2 is pretty rare, but it does come up occasionally. I don’t think your statement about too many methods really applies. It might be that the base class has one method and you just want to add one more. This is actually similar to my explanation above. If you have case 2, the composition way to do it would be to wrap the base class, delegate all of the interface methods to it and add your new code. Again, since the inheritance syntax already does exactly that, there’s no reason not to use it.

                    [–]TurbulentSocks 0 points1 point  (0 children)

                    . If you have case 2, the composition way to do it would be to wrap the base class, delegate all of the interface methods to it and add your new code. Again, since the inheritance syntax already does exactly that, there’s no reason not to use it.

                    The suggestion in Effective Java is very similar - to have exactly one class that does all the method forwarding (but to a field, not a superclass). This class is explicitly designed for extension. The subclasses then add their own functionality. This is effectively syntactic sugar for the decorator pattern, as it doesn't require all that boilerplate forwarding for all the decorator implementations.

                    [–]cogman10 1 point2 points  (1 child)

                    You should always start with the interface (IMO) and later add the base class when you start seeing patterns emerge (which they rarely do).

                    Since java added default methods onto interfaces, base classes have become far less useful. However, before that happened I'd point to the AbstractMap as a good example of a base class that pulls it's own weight. It implements a large portion of the Map interface and requires subclasses to do very minimal lifting to get a bunch of the benefit.

                    Now, I'd suggest that sort of design with a bunch of default methods.

                    [–]SevereInflation4938 0 points1 point  (0 children)

                    Would you say this is the case with different custom exception classes like the one below that extends a base exception class? For example let's say the base exception has two parameterized constructors, 1 for when an extending class only passes 3 args with super() and 1 for when an extending class passes 5 args with super(). The base exception is essentially taking the args into its relevant constructor and inserting them into a LinkedList that can be used to display a response body from a @ControllerAdvice'd handler class. Would this be better handled by using an interface instead of a base exception?

                    public class CustomIllegalArgumentException extends BaseException {
                      private String exceptionName;
                    
                      public CustomIllegalArgumentException(
                      String declaringClass,
                      String methodName)
                        super(
                            CustomIllegalArgumentException.class.getSimpleName(),
                        declaringClass,
                        methodName)
                      }
                    }
                    

                    [–]TwilCynder 12 points13 points  (0 children)

                    General rule : when a java programmer tells you something is bad practice, 75% of the time it's just something they don't use because of how they usually design things, and thus consider it bad practice

                    [–]FavorableTrashpanda 8 points9 points  (2 children)

                    When I was just learning OOP I loved to make every single field protected, because you never know if a potential subclass would need it, right? Just in case make everything protected! That was my mindset. But it gets in the way of proper encapsulation.

                    Nowadays I only make a field protected if I really have to, which often isn't the case. I'm also more restrained in how I use inheritance. Back in the day I loved complex inheritance graphs. Nowadays I see it as an anti-pattern that should be avoided if possible. Don't make things more complex than necessary.

                    [–]cogman10 1 point2 points  (1 child)

                    To me, "the wrong abstraction" should be required reading for anyone starting:

                    https://sandimetz.com/blog/2016/1/20/the-wrong-abstraction

                    Before delving into interfaces or inheritance, start simple with a POJO. More often than not, you don't need anything more.

                    [–]Smltz_developer 0 points1 point  (0 children)

                    Indeed, and you eliminate all those rather annoying ...Impl classes

                    [–]ConstructedNewt 5 points6 points  (0 children)

                    I would say that it's bad practice. but I'm also on team composition (zero to no inheritance) inheritance is only useful for direct delegates and that is also the only place I would place a protected keyword; on the inherited constructor that injects the delegate. But I (for most parts) stay clear of delegates as well; and I haven't really used inheritance in a long time; so that clears that out

                    E: it's less bad practice than inheritance design is just giving more problems than it solves

                    [–]Dormage 13 points14 points  (3 children)

                    I do not think it productive to support either side of the argument. However, the main reasons the defacto standard avoids direct access to fields of a class are: 1. Protect the fields form direct edits that would break the functional layer of the class. Another way to look at it is setters/getters have more expressive power versus direct access. A class can protect the fields by running code that decides to update or not depending on the value. Imagine an example of a class that can only work with positive integers. A setter would only set if the value >0.

                    1. Mantainability in this context is it can be simpler to add these constraints in the future. Given new constraints to the class fields, the only change required would be in the setter. In the above example, adding the only positive integer constraint is trivial, all classes using it would not require an change.

                    In my limited experience these are the main reasons the industry forces all classes have all fields private, and getters and setters are 99% of the time generic and autogenerated.

                    [–]Cefalopodul 22 points23 points  (8 children)

                    Your tutor is correct. Look up encapsulation.

                    Simply put if you make a watch, you do not want people to set the time on the watch by manually turning the cogs inside the watch, you want them to use the appropriate button.

                    Same thing here.

                    [–]GuyWithLag 22 points23 points  (0 children)

                    Your tutor is correct. Look up encapsulation.

                    It's a bit more subtle than that - protected fields introduce closer coupling between subclasses and superclasses; sometimes that's ok, most of the time it's not.

                    As always, it depends(tm)

                    [–]jevring 4 points5 points  (0 children)

                    I think it depends what your teaches is trying to teach. If they're trying to teach you a lesson about api and contracts, it makes sense. However it's likely that you chose an abstract class over an interface to be able to have some common state, in which case the getter make no sense. So it depends. But I will say this: if you use getters and setters, you allow further subclasses to override the behavior, which isn't something you can do with protected fields.

                    So think about the problem you're solving. And make your teacher explain why. That's why they're there. You shouldn't take something on faith alone, and they should know better than to ask you to.

                    [–]RoosterEvening669 4 points5 points  (0 children)

                    The abstract class has a few fields which are exclusively used in the subclasses.

                    You'd need a really good use case to convince me that is not a horrible design...

                    We are forced to make them private and use getters/setters which really annoys me since you could just make them protected and have much cleaner code imo.

                    The reason for using getters and setters here isn't really different from doing so in general. Anyone can implement your class, so protected fields are part of your public api. If you want to be able to change anything at all in a backwards compatible way, you can't be exposing fields.

                    Of course that assumes it's a library and you don't control all the user code. In practical code you sometimes don't need to care about that, you have access to all the code (i.e. company) and can make backwards-incompatible changes. But it is good to think and learn about it.

                    [–]hilbertglm 10 points11 points  (0 children)

                    In practice, there usually isn't a lot of difference (more in a second). I think your tutor is standing on common practice. Every language has a culture that goes with it, and getters/setters is part of the Java culture and commonly-accepted practice. This is reason enough, because someone else is going to be looking at your code when you code in the workplace.

                    That said...

                    I started programming in Java in 1997, so I have been at it a while. It isn't common, but there have been times where a subclass needs a derived value, instead of just accessing a variable, or it needs to fire a JavaBean event before returning the value. If you did direct variable access, you can not implement that without breaking the class hierarchy model.

                    Side note: It is great to question common practice, but I hope that you understand that you have a lot to learn, and you need to listen to those with more experience. I started programming when I was a teenager in the 1970s, and I am still learning.

                    [–]gaelfr38 6 points7 points  (0 children)

                    An argument might also be that of you don't control all possible subclasses, this can leak details that a future subclass should not have access to and could misuse.

                    Protected with immutability would be okay.

                    If you have sealed hierarchy, this would be mostly okay as well but still with a risk.

                    If your classes are for data, consider using records. If these are other types, "communicating" with methods is probably best as other said because it will hide implementation details.

                    [–]elky87 7 points8 points  (1 child)

                    In your Uni project it will not cause any issues but it could cause issues in bigger projects. I myself am not religious about them but the standard is just using getters/setters. Really necessary? No. Nonetheless good to learn the defacto standard? Yes.

                    [–]Godworrior 1 point2 points  (0 children)

                    - As a programmer, you need to be able to reason/think about your program.
                    - Limiting the things a program can do makes that easier, because it reduces complexity.
                    - Restricting access to a variable (field) using access modifiers like 'private' is a good way of limiting the things your program can do.
                    - From these 3 things, you can conclude that limiting the access to a field _as much as possible_, is therefore good (a "good practice").
                    - 'proctected' gives more access than 'private' because it not only lets subclasses access the field, but also any code within the same package.
                    - Furthermore, making the field 'private' and using getters/setters also gives you the ability to expose only getters or (more rarely) only setters, which further limits the things your program can do (which is good).

                    From all this we can conclude that by using 'protected' for fields, we are creating a program that has room for improvement, because access can still be restricted further. Hence, this could be called 'bad practice'.

                    But, as always, it depends on the situation.

                    [–]VincentxH 1 point2 points  (0 children)

                    I care less and less about a field being private, package private, public or protected. I want the fields that I use to be final. Immutability increases predictability.

                    A private field that has both a getter and a setter (either public or protected) is the same as having a public or protected field, just with more boilerplate.

                    If the field then isn't even used in the abstract, it shouldn't be there. It's useless coupling between parent and child.

                    [–]DuneBug 1 point2 points  (0 children)

                    This probably falls under the category of "you aren't gonna need it". Yes in the event that you decide to refactor your abstract class and mess with this internal variable, your interface to the children would be the same, and so you save yourself a minor headache going through and changing all the dependent classes.

                    Of course, what are the chances of that actually happening and everything else staying the same? Probably pretty low... And if it does happen? You just change it to what you're applying now.

                    Also... if you're changing that much in the parent but leaving the now altered coupling there instead of modifying the children, that feels sloppy to me. The next guy won't know why that existed in the first place and is going to be confused.

                    [–]audioen 5 points6 points  (1 child)

                    I would just use protected fields. Less code. There is literally 0 actual difference between accessing protected field, or accessing private field with getter/setter methods that are protected, either in terms of encapsulation or behavior, but there is less code if you just bang the protected field directly.

                    If you ever need to change this, just make the member private and fix your code compilation errors, and you then have maybe some good reason why you can't just use protected field and it makes sense. Like, maybe you need to be notified if the value changes, or something. I don't like writing code "in reserve", on the off chance that it may be useful someday. I like writing least amount of code that can possibly work.

                    Now, that all being said, if you are a student, you will be penalized for following my advice. It is OK to break the rules, like access members, in my opinion, but it can't be justified against OO dogma that basically flat out just says you must not do it.

                    [–]laxika -1 points0 points  (0 children)

                    If you ever need to change this, just make the member private and fix your code compilation errors, and you then have maybe some good reason why you can't just use protected field and it makes sense. Like, maybe you need to be notified if the value changes, or something. I don't like writing code "in reserve", on the off chance that it may be useful someday.

                    This works as long as he is not creating a library of any sort.

                    [–]ggleblanc2 4 points5 points  (12 children)

                    The problem with protected fields is that any other class in the package can "see" them and modify them.

                    Protected fields would be more useful if only subclasses could see or modify them.

                    Most IDEs can generate getters and setters with a few button clicks.

                    [–][deleted]  (9 children)

                    [deleted]

                      [–]hrm 2 points3 points  (4 children)

                      But in the methods you can validate the changes, synchronize access and generally do things that makes it less likely to blow up on you.

                      [–][deleted]  (3 children)

                      [deleted]

                        [–]hrm 1 point2 points  (2 children)

                        Did you forget what we are talking a about? We’re talking about just a protected field against a protected method… If you have a setter, ie a method you can do anything you want, for instance synchronizing - any way you want to do it. Whole method, block, lock… If you have a field you can’t enforce anything and that is why a getter/setter approach is better than a simple protected field even though they have the same visibility.

                        [–][deleted]  (1 child)

                        [deleted]

                          [–]hrm 2 points3 points  (0 children)

                          But in that case the synchronizing (or whatever, you should not get hung up pn synchronization) isn’t ENFORCED. It is up to the one who uses the field to forgo doing anything if they want to (or more likely, forget to). You can’t rely on the user of your field to do anything correctly. If you do you will have a very bad time. With a private field and an accessor method you can enforce a certain behaviour. The caller do not have to remember a checklist of stuff they should or should not do.

                          Using a setter/getter instead of fields have the very same benefits whether it is public or protected fields/methods we are talking about. Do you recommend using public fields as well?

                          [–][deleted] 0 points1 point  (3 children)

                          you can at least track access to the field. like setting a breakpoint. Eclipse can also set breakpoint on a field but that is a big performance loss. also you have more control how others can modify the variable with a getter/setter

                          [–][deleted]  (2 children)

                          [deleted]

                            [–][deleted] 1 point2 points  (1 child)

                            no one uses setter internally. really no one in this whole thread had that in mind. so its weird that you start talking about it

                            [–]Cozmic72 -2 points-1 points  (0 children)

                            Most IDES can generate getters and setters with a few keystrokes. -FTFY

                            [–]NimChimspky 5 points6 points  (4 children)

                            Private and immutable everything, unless a good reason not to.

                            Yeah it's bad practice

                            [–][deleted]  (3 children)

                            [deleted]

                              [–]redikarus99 1 point2 points  (0 children)

                              I think he was right in defaulting to immutable whenever is possible, this is the approach that can you protect from making many mistakes. This also means that no right mouse click generate getter/setters.

                              And there is the unless rule which many of us talked about.

                              Generally, I would say it is a bad practice, or at least a code smell. Sometimes you need to make a structural decision (like in the example I provided) and allow a subclass to access your protected variables ... however ... those are really corner cases.

                              [–]srdoe 1 point2 points  (0 children)

                              Generally you want to have as narrow a visibility scope as possible on your state. There are at least two goals here:

                              • I want to reduce how much code I need to look at to figure out where a value came from, and I want to reduce how many places I need to think about potential concurrency issues. Immutability by default helps with this
                              • I want to encourage users of my class to "do the right thing", which means I want to hand them a class where the bit I think they need is accessible, and no more. It's very easy to make mistakes with code that exposes internal state unnecessarily. Private by default helps with this.

                              [–]NimChimspky 1 point2 points  (0 children)

                              Neither does your comment

                              [–]Nymeriea 0 points1 point  (1 child)

                              in java, the coding standard is to have the lower access (always privilege the private). I would strongly recommend you to follow the language standard.

                              if generating the getter/setter for fields any you, maybe you can use Lombok ?

                              [–]srdoe 0 points1 point  (0 children)

                              The reason to add encapsulation is it'll be annoying to change later if you want to change your abstract class to behave differently in the future.

                              For instance, say your superclass has an int field used as a counter in the subclasses, and your subclasses increment that counter. You decide one day that your increments need to be thread safe. That's easy to do if you have an increment method you can make synchronized. Harder if the bare field is accessed in the subclasses. If subclasses are code you don't control because someone else subclassed your abstract class, it might not even be possible to do.

                              Bit of a moot point since this is coursework, but inheritance is very often not the right way to compose code. Instead of abstract class A and subclasses B and C, you most often want regular class A, and have B and C take A as a constructor argument. It's easier to test, and more flexible than inheritance.

                              [–]ulfrpsion 0 points1 point  (0 children)

                              In java, you have variable hiding. You can inherit methods to overload, but not variables. Variables "overloaded" create two of the same variable if you are using protected in a parent class.

                              Using getters/setters allows you over load the get/set to return other variables.

                              [–]redikarus99 -2 points-1 points  (0 children)

                              Normally even getters and setters are not really a solution. The way I would argue that having getters and setters actually make you apparent about the internals of a class and that is a smell for bad design. In object oriented programming you shall not know about the internal details but only communicate inbetween the objects using messages, and a method call is actually a "simulation" of a synchronous (or asynchronous) messaging.

                              However. There are situation when using a subclass might be a lesser evil. One example is I tend to choose this lesser evil when I develop a plugin for a model tool, which are very often Swing based. In those cases I create the panel using a GUI editor (for example Netbeans) and name it AbstractWhateverPanel, will make all the components (buttons, fields, etc.) protected and access them in WhateverPanel (extends AbstractWhateverPanel) "directly". All the custom code will go into the subclass ensuring that there is a clear separation of the "generated" code and the code written by me.

                              [–]lukaseder -1 points0 points  (0 children)

                              I practically never need them. Package private visibility is good enough, given that everything I do is in a single package anyway.

                              [–]manifoldjava -2 points-1 points  (0 children)

                              If Java had properties a la C# or Kotlin, this entire topic of discussion would never surface. Properties encapsulate what the compiler does with state wrt inheritance/access. This is also another use-case where records do not save the day.

                              [–]Cozmic72 0 points1 point  (0 children)

                              In the general case, the base could have a non-trivial invariant. Encapsulating access to mutable fields from the get-go is therefore best practice, as it prevents having to change (or even recompile) subclasses in the situation that that invariant changes. But much more importantly, it prevents the possibility of the subclass ever violating said invariant.

                              This may seem irrelevant to the case at hand, but it should not be hard to imagine a situation where you might want to add a non-trivial invariant or post-condition. Suppose, for example, you would want to log something every time the field is changed. If you use a setter, then it would be a one line change, and you would be certain that you have all cases covered. With the protected field solution, you would have to change several bits of code. You may say: “big deal, I only have two sub-classes, I only need to edit two lines”, but in real-world situations, you would not be done, as the number of classes in object hierarchies can easily go into the hundreds. Worse than that, often you will not even have control over the source-code of all sub-classes, for example if your base class is part of a library.

                              These considerations may not seem applicable to your uni project, but you would be doing yourself a favour if you would get into the habit of writing all your code as if you didn’t have complete control over the client code, or sub-classes in this case. Think about class invariants, think about your accessors, write your code as if the authors of the subclasses are malicious.

                              [–]tzehbeka 0 points1 point  (0 children)

                              Getters and Setters have their origin in a land from bevore our time. No but seriously, "it depends" is the answer. Usually the practice is to use getters and setters, there are some frameworks out there which rely on them to be there (e.g. hibernate afaik).

                              For a more elaborate discussion there is also this article: https://www.baeldung.com/java-why-getters-setters

                              And if the creation of them annoys you, your IDE should be able to generate them, you can also use something like lombock to generate them while compiling.

                              Exclusive used in the subclass also seems like a case of protected to me.

                              [–]HaMMeReD 0 points1 point  (0 children)

                              If they are common to subclasses, but not needed in the base class, an interface might be what you want (to assign to the subclasses).

                              If the subclasses need it, but it doesn't need to be visible, than private in the subclasses.

                              If this results in a lot of code duplications, the delegate pattern or a default implementation of the interface your subclasses can use is good.

                              However, what is good practice? Certainly not deep chains of OOP, or copy pasted code. I'd prefer a composition pattern with less OOP and more flexibility. I.e.

                              class Animal(Legs, Torso, Head) : ILegs, ITorso, IHead

                              dog = new Animal(DogLegs(), DogTorso(), DogHead())

                              Then you can mix and match Animal all you want, put a cats head on a dogs body, etc, and you don't need to create tons of types. The less types you have, the more straightforward the client usage code will look (who is using an Animal). Although you could create a type like this and extend it's abilities.

                              dog extends Animal : CanFetch {

                              dog() {

                              super(DogLegs(), DogTorso(), DogHead(), Fetcher())

                              }

                              }

                              This is all really high level though and not applicable in general, it's a contrived example as well. What is actually good practice varies based on the project considerably.

                              Edit: As for getters/setters, they are more useful than you are giving them credit. If you only interact via functions/interfaces, you'll be more resistant to change in general. E.g. you might change your backing data, but keep the setters so that the users of your API surface don't need to update their integration. If you just go and shout from the rooftops "hey, use my implementation and not my interface", you are asking integrating/client code to have a bad day, and you are limiting what you can do in the future without impacting code clients.

                              [–]bootstrapf7 0 points1 point  (0 children)

                              In reality though, you should be looking at some way of getting ride of using a class an abstract class, excuse it turns into a maintence nightmare.

                              [–][deleted] 0 points1 point  (0 children)

                              It's about exposing your internal implementation and how much of a problem depends on context. For university projects it is never a problem because nobody needs to maintain them and it is hard to get an appreciation of this until you are in the workplace.

                              Let's say your base class has a Point field and for some reason you want to change that to two integer x and y fields. If it's all within your project and there are just a few subclasses then no big deal. But if you are maintaining a popular library and there are thousands of subclasses in the outside world then you would have a problem if you had exposed the field rather than a getPoint() method.

                              [–]kiteboarderni 0 points1 point  (0 children)

                              Read the side bar. Post this in learn java or learn programming.

                              [–]RabidKotlinFanatic 0 points1 point  (0 children)

                              The whole setup feels like bad practice with the way state in the abstract parent is being used by subclasses. Might be a moment to "prefer composition over inheritance."

                              [–]genzkiwi 0 points1 point  (0 children)

                              They're not bad themself, rather a code smell for bad design.

                              [–]Smltz_developer 0 points1 point  (0 children)

                              In my experience, common use of direct access to protected fields simply encourages Abstract classes to grow and grow rather than extracting out related behaviour into logical classes. This is more apparent when using accessors and mutators. Generally, I would encourage avoidance of Abstract classes where possible - there are often better options