[ 
https://issues.apache.org/jira/browse/MAHOUT-190?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12770588#action_12770588
 ] 

Sean Owen commented on MAHOUT-190:
----------------------------------

I agree there's a tension between closed-for-modification and 
open-for-extension. I believe there's more harm in allowing extension without 
designing for it than the opposite. And in the code so far, I really don't see 
evidence of explicit design for extension. I see overrideable methods in 
constructors, etc.

When confronted with a class that could be either closed up to be safe, or 
designed for extension, I'm all for designing for extension.

But does that ever involve having non-private fields? That's the narrower 
question I'm asking.

I do think a getter is better than a raw field access, yes. I don't agree that 
every field should be accessible by default, no, which is why it makes any 
difference. Something should be accessible on purpose, not by accident, and I 
don't sense many of the non-public fields in the project now are on purpose.

Replacing it with a getter does little, yes, but it's a step in the right 
direction? Do we have evidence there is a problem with people wanting to extend 
and not being able to?

I'd also advance the opposite problem: you have a protected field that the dev 
really doesn't think of as something used by anyone. He removes or changes it. 
Oops, just broke two subclassers. Either he reverts and realizes, actually, 
that's now part of the API, sorry, or finds some workaround (like migrating to 
getters). Is that a lesser problem?

> 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.

Reply via email to