I've laid in a ticket:
https://issues.apache.org/jira/browse/JENA-938
and attached a few PRs of reasonable size. They contain the removal of
superinterfaces that don't need declaration, checked exceptions that cannot be
thrown, and unnecessary typecasts. Those seemed to be entirely
non-controversial moves to make. They result in additions and deletions, but
the net result is many, many lines that are shorter and easier to read and 439
fewer LOC in total.
It's not clear to me that there was consensus about removing never-called
private methods, unreachable code (i.e. "if (false) {…}") or unused fields.
I think I could send in at least one more PR with the removal of unused local
variables? That also seems generally non-controversial.
---
A. Soroka
The University of Virginia Library
On May 8, 2015, at 10:14 AM, Claude Warren <[email protected]> wrote:
> I think the catch of execptions that can not be thrown can be safely
> removed.
> I would also vote up removal of private methods that are never used.
>
> fields are a bit trickier, but then I am probably thinking of parameters
> and matching an interface... Yeah, I would vote up removing unused methods.
>
> Claude
>
>
> On Fri, May 8, 2015 at 3:54 AM, [email protected] <[email protected]>
> wrote:
>
>> I'm building a PR [1] right now as a sort of "think-piece" to give us
>> something concrete to look at. I'm building it up out of ONLY things that
>> Eclipse/javac can determine are definitely impossible to execute or
>> redundant or never used, including:
>>
>> - Private methods that are never used.
>> - Superinterfaces that don't need declaration.
>> - Fields that are never used.
>> - Checked exceptions that cannot be thrown.
>>
>> and so far, I'm at about 11,000 lines to delete, which is… a good many.
>> Certainly too many to believe that all are really totally dead stuff that
>> should be gone. {grin} As you point out, some portion of this is stuff that
>> we wouldn't want to lose. My hope is that we can look over this PR and
>> develop some tickets for the kinds of things to which you refer ("e.g.
>> features that didn't make it into SPARQL") and insert some TODOs and so
>> forth. And maybe we can use it as a starting place for actual pruning. I'll
>> send the PR sometime tomorrow.
>>
>> [1] https://github.com/ajs6f/jena/tree/KillDeadThings
>>
>> ---
>> A. Soroka
>> The University of Virginia Library
>>
>> On May 7, 2015, at 7:07 PM, Andy Seaborne <[email protected]> wrote:
>>
>>> +1 to removing dead code though what is "dead" is tricky. In arq and tdb
>> there was some but they included code that is a useful record (e.g.
>> features that didn't make it into SPARQL). I removed obvious junk. Some is
>> checking code that I'd like to leave.
>>>
>>> I had a look - a regex of "if *\( *false *\)" but I didn't find much in
>> core (just 2)
>>>
>>> "if(false)" requires the compiler to generate no code and "final
>> boolean" but in Java8, does that include effectively final?
>>>
>>> What were you looking for?
>>>
>>> I tend to agree that the use of a field makes things worse.
>>>
>>> Andy
>>>
>>> On 07/05/15 19:24, Stephen Allen wrote:
>>>> I'd say just eliminate all of that dead code. Also any commented code
>> as
>>>> well. We have a source control system, one can always look into the
>>>> history to get that stuff. Using a field just makes it worse IMO...
>> it'll
>>>> never get removed if we do that.
>>>>
>>>> -Stephen
>>>>
>>>>
>>>> On Thu, May 7, 2015 at 11:26 AM, A. Soroka <[email protected]> wrote:
>>>>
>>>>> There are a goodly number of pieces (>150) of "dead code" in Jena, of
>> the
>>>>> form:
>>>>>
>>>>> org.apache.jena.mem.HashCommon:
>>>>>
>>>>> void showkeys()
>>>>> {
>>>>> if (false)
>>>>> {
>>>>> System.err.print( ">> KEYS:" );
>>>>> // some logging code
>>>>> System.err.println();
>>>>> }
>>>>> }
>>>>>
>>>>> If I understand this rightly, these are cases where we want to keep
>> some
>>>>> code "on deck" for potential use. I'd like to suggest that many of
>> these
>>>>> guys might be rewritten with a field or fields in the class, something
>> like:
>>>>>
>>>>> boolean useLoggingCode = false;
>>>>>
>>>>> void showkeys()
>>>>> {
>>>>> if (useLoggingCode)
>>>>> etc.
>>>>> }
>>>>>
>>>>> This would make things a bit clearer and clean out a bunch of compiler
>>>>> warnings.
>>>>>
>>>>> Does this sound like a good approach? Worth doing?
>>>>>
>>>>> ---
>>>>> A. Soroka
>>>>> The University of Virginia Library
>>>>>
>>>>>
>>>>
>>>
>>
>>
>
>
> --
> I like: Like Like - The likeliest place on the web
> <http://like-like.xenei.com>
> LinkedIn: http://www.linkedin.com/in/claudewarren
