Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jdt-ui-dev] Proposal: Refactoring "Remove unused members"

Olivier,

  good point, I'll keep that in my mind for the next version of the proposal. 
This might also effect protected members as well.

Cheers,
Mariano

On Tuesday 15 July 2003 00:40, Olivier Dagenais wrote:
> Hi,
>
> Just a quick note:  it may not be safe or polite to simply remove public
> members right away, especially if you're working on a library.  I would
> recommend there be an option for labelling those as "deprecated" instead of
> removing them.  Bonus points, then, for a command that searches and removes
> only deprecated members. (for the next release)
>
> I don't know if Eclipse's Java tools already do this (I'm new here), but
> just a use case to consider...
>
> - Oli
>
> ----- Original Message -----
> From: "Mariano Kamp" <mkamp@xxxxxx>
> To: <jdt-ui-dev@xxxxxxxxxxx>
> Sent: Sunday, July 13, 2003 11:44
> Subject: [jdt-ui-dev] Proposal: Refactoring "Remove unused members"
>
> > Hi,
> >
> >   the idea is to remove members (classes, methods and class/instance
> > variables) which are not referenced (anymore). The collected candidates
>
> for
>
> > removal are to be displayed to let the user pick the methods s/he wants
> > to remove.
> >
> >   Hopefully I was able to express myself clearly. If not, please ask. I
> > am more than happy to try a different approach of explaining or thinking
>
> about
>
> > it.
> >
> >   I believe at least the following Issues have to be taken care of...
> >
> > [ Simple Example ]
> >
> > Let's start with one of the simplest cases.
> >
> >       public class A {
> >         private void a() {}        // referenced from inside this class
> >         public  void b() { a();} // referenced from the outside?
> >         private void c() {};       // never referenced
> >       }
> >
> > This would result in the following collection of items to analyze:
> >
> > Name           Containing Type             Access
> > of Item          and Supertypes              Modifier
> >                       declaring the item
> > ------------------------------------------------------------
> > a()                    A                                     private
> > b()                    A                                     public
> > c()                    A                                     private
> >
> > The results of this case are pretty obvious:
> >
> > A.b() is not referenced from inside A. According to the public access
>
> modifier
>
> > of A.b() the whole workspace has to be searched for items referring to
>
> this
>
> > method. If none is found it is a candidate for removal.
> > A.a() is referenced from A.b(). Assuming A.b() is referenced from the
>
> outside
>
> > of this class, it will _not_ be a removed.  Hence A.a() will _not_ be a
> > candidate for removal.
> > A.c() is not referenced from inside A, there is no named inner class and
>
> as
>
> > A.c() is declared private the search ends here. Hence it _is_ a candidate
>
> for
>
> > removal.
> >
> >
> > [ Taking Supertypes into Account ]
> >
> > An example with three super types:
> >
> >       interface I {
> >         void i();
> >       }
> >       public class SuperSuperA {
> >         public void ssaa() {}
> >         public void ssab() {}
> >       }
> >
> >       abstract class SuperA extends SuperSuperA
> >                           implements I {
> >         void i() {}
> >       }
> >
> >       public class A extends SuperA {
> >         public void i() {}
> >         public void ssab() {}
> >       }
> >
> > Name     Containing Type  Access    Remark
> > of Item    and Supertypes   Modifier
> >                 declaring
> >                 the item
> > -------------------------------------------------------------------------
> >-
>
> -----------
>
> > i()            A, SuperA, I          public      The  implementation of
> >                                                                 A.i()
>
> could be
>
> >                                                                 used 
> > when referring
>
> to
>
> >                                                                 SuperA or
>
> I.
>
> > ssab()    A, SuperA,             public      The implementation of
> >                                                                 A.ssab()
>
> could
>
> >                                                                 be
>
> SuperSuperA
>
> >                                                                 used when
> >                                                                 referring
>
> to
>
> >                                                                 SuperA or
>
> SuperSuperA.
>
> > Note that SuperSuper.ssaa() didn't make it into the collection of items
> > as
>
> it
>
> > is not overridden in A.
> >
> > Resulting in:
> > As A.i() is public the whole workspace has to be searched for references
>
> to
>
> > A.i(), SuperA.i() and I.i().
> > This is also true for A.ssab(). A.ssab(), SuperA.ssab() and
>
> SuperSuperA.ssab()
>
> > have to be searched in whole workspace.
> >
> >
> > [ Recursive Dependencies  ]
> >
> > An example with recursive dependencies.
> >
> >       public class A  {
> >         private void a() { b(); }
> >         private void b() {}
> >       }
> >
> > A.b() can not be a candidate for removal as it is referenced by A.a().
>
> A.a()
>
> > can be removed though, but after doing so, A.a() is also a candidate for
> > removal. As the user may not select to delete A.a(), A.b() cannot be
>
> removed
>
> > in every case. (any case? my English su... I mean sometimes it can,
>
> sometimes
>
> > it can't).
> >
> > Therefore it is necessary to record the candidates differently, including
>
> the
>
> > dependencies.
> >
> > Name          Containing Type         Access            Depends on
> > of Item         and Supertypes          Modifier           the removal of
> >                      declaring the item
> > -------------------------------------------------------------------------
> >-
>
> ------------------
>
> > a()                     A                               private         -
> > b()                     A                               private
>
> A.a()
>
> > The collection of removal candidates is to be presented to the user in a
>
> way
>
> > reflecting the dependencies.
> > One possible implementation would be to show the candidates in a
>
> hierarchical
>
> > view. The independent candidates are shown on the first level and the
> > respective depending items are below the candidate they depend on. If a
> > containing candidate is deselected all depending candidates and
> > candidates depending on the latter are also deselected and cannot be
> > selected without selecting the candidate they depend on.
> >
> > A typical example for recursive dependencies might be accessors and the
> > respective instance variable or (inner) classes which are only referenced
>
> by
>
> > a Widget using them as XXListeners and can be removed when the button
>
> reaches
>
> > its end of life.
> >
> >
> > [ When to stop searching for References ]
> >
> > Please consider the following code.
> >
> >       public class A  {
> >         public static void aa() { }
> >         public static void ab() { A.aa(); }
> >                 // main() and all methods
> >                 // of Object are blacklisted
> >                 // and will not be used as
> >                 // candidates.
> >         public static void main(String[] args) {A.ab(); }
> >       }
> >
> >       public class B {
> >         public static void ba() { A.aa();}
> >       }
> >
> >       public class C {
> >         public static void ca() { A.aa();}
> >       }
> >
> > Assumption: As A.aa() is referenced by A.ab(), B.ba() and C.ca() it is
> > not
>
> a
>
> > candidate for removal.
> > To get this result the whole workspace must be searched as A.aa() is
>
> declared
>
> > public, but this would mean a serious performance penalty.
> > The first possible optimization would be to stop looking for more
>
> references
>
> > after the first is found. In the am scenario that would mean to stop to
>
> after
>
> > A.ab() is found. As a consequence A.aa() would be in a unknown state when
> > A.ab() (another candidate) will not be confirmed by the user, because
> > when A.ab() is not removed it is unknown if A.aa() is referenced by more
>
> callers
>
> > than A.ab(), as the search has been stopped prematurely.
> > Therefore the search can only be stopped, after the first caller is found
> > which is not also a candidate.
> > Hence the callers, which are also candidates, have not to be recorded in
>
> case
>
> > a caller outside the candidate list is found.
> >
> >
> > [ Performance ]
> >
> > I believe that displaying the search results and executing the removal
> > are relatively cheap compared to the search, so I will just focus on the
>
> search
>
> > for now.
> >
> > - general search scope -
> > For non-static private members only the class itself and the named,
> > non-static, inner classes have to be searched.
> > The search for members declared with the package modifier will start like
>
> a
>
> > search for private members, but will also the search the declaring
>
> package.
>
> > Regarding protected members the private and package search has to be
>
> performed
>
> > and furthermore all subclasses, which are not in the same package as the
> > member itself.
> > Public members are the worst. They can be referenced by everything. So as
> > worst case the whole workspace has to be searched.
> > This is just a rough outline to build on for the below mentioned issues.
> >
> > It would be important to show the user a tip helping him to understand
>
> that it
>
> > is important to define an appropriate workingset, which includes only
>
> his/her
>
> > own code, not all the libraries etc.
> >
> > Also the default settings should already narrow down the search, e.g.
>
> "public"
>
> > as a search criteria should be deselected.
> >
> > - batching the search -
> > The two factors having the most impact on the performance will likely be
>
> disk
>
> > io and parsing, therefore an ideal search would touch each ressource only
> > once and also parse it only once.
> >
> > - stopping early / redefining search scope -
> > That would mean that search scope needs to reflect the maximum scope
>
> universe
>
> > of all members to search for. If there are 10 members to search for
>
> declared
>
> > private and one public this could mean to search the whole workspace.
>
> Still
>
> > for this particular public member it is the only choice there is.
> > Therefore it is necessary to suspend the search after the first reference
>
> to
>
> > the public member is found (where the calling member is not a candidate
> > itself) _and_ recalculate the maximum search scope for the remaining
> > candidates (in the a.m. case it should be none, as private members should
>
> be
>
> > searched first, but there are other cases, like having more than one
>
> public
>
> > member to search for).
> > To make a long story short, the search scope needs to be redefined during
>
> the
>
> > search and it also must be possible for the caller to stop the search for
>
> the
>
> > case that relevant references are found for all candidates before having
> > searched the remaining scope completely.
> >
> >
> > [ Performance :: Implementing the search using the Java Search Engine ]
> >
> > I had a quick peek at the JavaSearchEngine and although I am not through
>
> yet,
>
> > I have found the OrPattern to build the maximum search scope, but haven't
> > found anything yet to suspend/stop a running search and to reduce the
>
> scope
>
> > of it.
> >
> > _______________________________________________
> > jdt-ui-dev mailing list
> > jdt-ui-dev@xxxxxxxxxxx
> > http://dev.eclipse.org/mailman/listinfo/jdt-ui-dev
>
> _______________________________________________
> jdt-ui-dev mailing list
> jdt-ui-dev@xxxxxxxxxxx
> http://dev.eclipse.org/mailman/listinfo/jdt-ui-dev



Back to the top