Re: [EGIT PATCH] Comment private modifier to improve performace.

["Roger C. Soares" <rogersoares@intelinet.com.br>]

Robin Rosenberg escreveu:
>
> I'm not fully convinced this is the right way after all. Good
> performance is obviously good, but so is good encapsulation. 
> I've sometimes tried changing things like this even in pieces
> of code that I really thought it should matter, but not been able
> to measure any real improvemen even with performance measurment tools.
>
> Obviously seeing that warning is annoying so maybe we should just set it to
> ignore or exclude it from the project settings (if that is possible). The
> only project where I think it might make a difference is the jgit part because
> that is where we optimize and that is where I experimented with visibility
> changes. In the Eclipse part we need to encapsulate more, partly because 
> Eclipse is less understood by the current authors than Java in general. 
> Encapsulation means encapsulating bad coding and bad design that comes
> from lack of understanding of the framework we are working within.
>
>   

Ok, so some more points for your consideration:

. I saw this /* private */ notation on eclipse code and found it 
interesting.

. I won't bother measuaring any single case to make sure it is not 
impacting performance under some circunstance, so resolving those 
warnings puts me in the safe area. On the other hand, I think it is a 
lot easier to tell if a patch is breaking encapsulation in a bad way 
just by reviewing it, which is something that is already done. 
Especially if it has the private modifier commented out. Someone can 
even do a script to uncomment them and verify that it still builds 
without errors.

. The ui part isn't supposed to be reused by other projects, so I think 
encapsulation there is less important than for jgit. But even so, the 
default modifier (or package-private) is good enough for encapsulation. 
Other projects shouldn't write code in the the same packages from jgit, 
if they do so they know that they are using internal things and they can 
run into problems in the future.

That said, I'm ok with your patch too. I think the important is to 
choose one so we stop mixing things, for consistency's sake.

[]s,
Roger.