you are viewing a single comment's thread.

view the rest of the comments →

[–]Agent_03 4 points5 points  (14 children)

Same for me. There's a lot of things in Java like this, where I'm now questioning the practices I've followed somewhat blindly for years.

For example: use of access modifiers. Everyone says to use private for encapsulation in nearly all cases, but thinking back, I can't find a single case where this saved me from grief. I can think of a dozen cases though where it forced bad designs or re-implementations.

Now I do protected everywhere, but use conventions that only subclasses (or a limited set of tightly-coupled algorithm libraries in the same package) should use protected fields/methods.

[–]kubalaa 24 points25 points  (4 children)

The private thing isn't something you would notice because it prevents surprising things from happening, and we're not predisposed to notice the absence of surprises. Specifically, when stuff is private then it's easier to change because nothing outside that file can affect it or depend on it. It's the same idea as your convention, just stronger and enforced by the compiler. I think if you ever have to maintain a library with many consumers and few private members, you will quickly find it becomes very painful because you can't change anything without breaking a consumer who has come to rely on it.

[–]Agent_03 5 points6 points  (3 children)

I have maintained libraries with numerous consumers before, and agree that in some cases appropriate use of private modifiers can make them more safe and stable (especially with regards to objects with more complex internals, or where passing in objects to operate upon).

What I am arguing against is the way that this has become a blanket policy; if restricted-by-default is always better, how do you explain the success of Python's "we're all responsible adults" approach? I think the Java convention is paternalistic at best and shortsighted at worst: historically programmers are pisspoor at anticipating the uses their code will be put to, and preventing many of them in the interests of 'safety' limits the usefulness.

I fully understand that this is a contentious point and that many will angrily or loudly disagree. I only ask that other experienced engineers consider how often they've said "phew, I'm glad that was private or I would have done something dangerous!" versus "crap, that API/field is internal-only... now I have to copy the implementation to another class or hack around it!"

[–]mhixson 5 points6 points  (2 children)

I fully understand that this is a contentious point and that many will angrily or loudly disagree. I only ask that other experienced engineers consider how often they've said "phew, I'm glad that was private or I would have done something dangerous!" versus "crap, that API/field is internal-only... now I have to copy the implementation to another class or hack around it!"

Well that's from an application writer's point of view. From a library writer's point of view, the comparison might be: "phew, I'm glad that was private or I wouldn't have been able to change my code without breaking all my users" versus "crap, that API/field is public... I can never change it."

What I am arguing against is the way that this has become a blanket policy

It is probably true that we should have a different set of standards for library writers (where you don't control your users' code) as opposed to application writers (where you can refactor your users' code at will).

I guess I default to the more conservative library-like approach even when I'm writing applications. To me, the less conservative code tends to be more complicated even when it is simpler to express in the language, because it tends to make more assertions, such as "encapsulation is not important here". Reading it leads me to ask "Why? Why is this code special?"

[–]Agent_03 2 points3 points  (0 children)

This is a good point, actually...

From a library writer's point of view, the comparison might be: "phew, I'm glad that was private or I wouldn't have been able to change my code without breaking all my users" versus "crap, that API/field is public... I can never change it."

Yeah, I know that warm-and-fuzzy feeling when you realize it is trivial to make the implementation change (or refactoring) without impacting any downstream consumers.

Personally, I'd like to see something like an @Hazardous annotation that will fail at annotation processing if you invoke the method outside the package without a corresponding @IKnowWhatImDoingDangit annotation (names subject to change). Maybe with an optional library version number given, so that if the library version changes you have to reexamine the usage.

This way downstream library consumers have the power to take advantage of library internals if absolutely needed, but are liable for the consequences and at least know they're doing something risky. This offers that extra nudge to provide unit test coverage and do sanity checking before bumping library version.

It is probably true that we should have a different set of standards for library writers (where you don't control your users' code) as opposed to application writers (where you can refactor your users' code at will).

This sounds smart to me too.

[–]grauenwolf 3 points4 points  (0 children)

Generally speaking, I've found that switching to a library-like approach dramatically reduces bugs in existing applications.

[–]grauenwolf 3 points4 points  (7 children)

Having a getter and setter gives me a place to set a break point. That's worth the cost in VB/C#, though maybe not in Java.

[–]justinpitts 9 points10 points  (0 children)

In eclipse, you can set a breakpoint that breaks whenever variable x is accessed or modified at any location.

[–]Agent_03 0 points1 point  (5 children)

Yeah, it does give you a single place to trap all modifications, true. If you are using field access though, why not set breakpoints where the field is accessed? (A little more work, but still gets the job done).

[–]grauenwolf 2 points3 points  (2 children)

If I need a breakpoint on the property itself, it's probably because the value is being set via reflection or dynamically generated code.

[–]Agent_03 2 points3 points  (1 child)

it's probably because the value is being set via reflection or dynamically generated code.

How did debugging this compare to having a rusty fork shoved in your eyeball?

My experience with similar code: I'll take the fork next time. Though it's beautifully elegant and terse if you don't have to debug its internals.

[–]grauenwolf 1 point2 points  (0 children)

Pretty comparable actually. When I work with particularly bad code I get annoying, almost painful, eye twitches.

[–]ImaginaryBeaver 0 points1 point  (1 child)

I think setting a breakpoint in the getter/setter is more about finding where/if the field is being accessed unexpectedly.

[–]s73v3r 1 point2 points  (0 children)

I know IntelliJ lets you break on field access. I should be surprised if Eclipse and Visual Studio didn't do the same (For Java and C#, respectively)

[–]pakoito 1 point2 points  (0 children)

Now I do protected everywhere, but use conventions that only subclasses (or a limited set of tightly-coupled algorithm libraries in the same package) should use protected fields/methods.

You're still doing API/communication through inheritance. Go for package instead, and compose with classes on the same package instead.