[
https://issues.apache.org/jira/browse/MAHOUT-190?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12770871#action_12770871
]
Sean Owen commented on MAHOUT-190:
----------------------------------
Yes, I am suggesting we (= I) go through now and create potentially all the
getters/setters. It will take me 10 minutes with my IDE.
My personal preference is strongly to design for extension, but failing that,
prevent extension if it's not designed for. and a lot of stuff is not yet
designed for extension.
I am surprised to hear we'd welcome some dependencies to weave their way into
the internal representation of these classes, in ways we aren't tracking. Tens
of small subtle bugs come to mind. Oops, now I want to synchronize on some
internal object. But I've allowed callers to access it directly, and they are
too. Maybe a deadlock occurs. Oops I didn't expect the field to be nulled at
this point.
Isn't just opening up the representation just punting on designing for
extension? should it not be intentional?
The strong argument for complete extensibility sounds like an argument for no
encapsulation, which can't be the idea. There's a line, and I thought
encapsulating representation was one of the things farthest from that line. I
am sure that's the right thing given my own experience, but we all have
different experience, and I'm not pushing this point of view.
One other thing, it's open-source right? this is the very case where the
worst-case is just that someone copies/pastes a class. It's not a closed
library.
The least change would be to expose absolutely everything through
getters/setters. I think you said it Jake -- it's a crazy lot of methods added,
most of which are not necessary. But these 'methods' already exist, they're
part of the API, in the form of accessible fields. They're in the javadoc. This
change is just a 'messenger'.
Why don't I make a patch that does in fact add all the getters/setters, for a
look. I think in many cases it will just highlight that the fields aren't going
to be useful to any extenders. And we chuck those. And we leave a lot of them.
And in 3 versions, can even review them again.
> Make all instance fields private
> --------------------------------
>
> Key: MAHOUT-190
> URL: https://issues.apache.org/jira/browse/MAHOUT-190
> Project: Mahout
> Issue Type: Improvement
> Affects Versions: 0.2
> Reporter: Sean Owen
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.3
>
>
> This one may be more controversial but is useful and interesting enough to
> discuss.
> I personally believe instance fields should always be private. I think the
> pro- and con- debate goes like this:
> Making all fields private increases encapsulation. Fields must be made
> explicitly accessible via getters and setters, which is good -- default to
> hiding, rather than exposing. Not-hiding a field amounts to committing it to
> be a part of the API, which is rarely intended. Using getters/setters allows
> read/write access to be independently controlled and even allowed -- allows
> for read-only 'fields'. Getters/setters establish an API independent from the
> representation which is a Good Thing.
> But don't getters and setters slow things down?
> Trivially. JIT compilers will easily inline one-liners. Making fields private
> more readily allows fields to be marked final, and these two factors allow
> for optimizations by (Proguard or) JIT. It could actually speed things up.
> But isn't it messy to write all those dang getters/setters?
> Not really, and not at all if you use an IDE, which I think we all should be.
> But sometimes a class needs to share representation with its subclasses.
> Yes, and it remains possible with package-private / protected getters and
> setters. This is IMHO a rare situation anyway, and, the code is far easier to
> read when fields from a parent don't magically appear, or one doesn't wonder
> about where else a field may be accessed in subclasses. I also feel like
> sometimes making a field more visible is a shortcut enabler to some bad
> design. It usually is a bad smell.
> Thoughts on this narrative. Once again I volunteer to implement the consensus.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
