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

From: Roger C. Soares <rogersoares@intelinet.com.br>
Date: Feb 3, 2008, 19:46 UTC
Message-ID: <47A61A30.3000904@intelinet.com.br>
In-Reply-To: <200802030201.10971.robin.rosenberg@dewire.com>

Robin Rosenberg escreveu:

Show 17 quoted lines

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