You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by "A. Soroka" <aj...@virginia.edu> on 2015/05/07 17:26:16 UTC
another clean up suggestion: dead code and its resuscitation
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
Re: another clean up suggestion: dead code and its resuscitation
Posted by "ajs6f@virginia.edu" <aj...@virginia.edu>.
There is now a PR at:
https://github.com/apache/jena/pull/58
with much of this work available. The idea is not to merge that gargantuan PR, but to give folks an easy way to see what the code looks like after I took a meat ax to it. {grin}
I would be happy to create more reasonable packages of changes from that monster PR for serious review and possible merging. Would a module-by-module approach be best?
---
A. Soroka
The University of Virginia Library
On May 8, 2015, at 10:14 AM, Claude Warren <cl...@xenei.com> 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, ajs6f@virginia.edu <aj...@virginia.edu>
> 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 <an...@apache.org> 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 <aj...@virginia.edu> 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
Re: another clean up suggestion: dead code and its resuscitation
Posted by "ajs6f@virginia.edu" <aj...@virginia.edu>.
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 <cl...@xenei.com> 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, ajs6f@virginia.edu <aj...@virginia.edu>
> 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 <an...@apache.org> 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 <aj...@virginia.edu> 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
Re: another clean up suggestion: dead code and its resuscitation
Posted by Claude Warren <cl...@xenei.com>.
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, ajs6f@virginia.edu <aj...@virginia.edu>
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 <an...@apache.org> 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 <aj...@virginia.edu> 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
Re: another clean up suggestion: dead code and its resuscitation
Posted by "ajs6f@virginia.edu" <aj...@virginia.edu>.
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 <an...@apache.org> 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 <aj...@virginia.edu> 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
>>>
>>>
>>
>
Re: another clean up suggestion: dead code and its resuscitation
Posted by Andy Seaborne <an...@apache.org>.
+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 <aj...@virginia.edu> 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
>>
>>
>
Re: another clean up suggestion: dead code and its resuscitation
Posted by Stephen Allen <sa...@apache.org>.
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 <aj...@virginia.edu> 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
>
>