\newglossaryentry{designPattern}
{
name={design pattern},
- description={A design pattern is a named abstraction, that is meant to solve a
+ description={A design pattern is a named abstraction that is meant to solve a
general design problem. It describes the key aspects of a common problem and
identifies its participators and how they collaborate},
plural={design patterns}
\noindent This definition assigns additional meaning to the word
\emph{refactoring}, beyond the composition of the prefix \emph{re-}, usually
meaning something like ``again'' or ``anew'', and the word \emph{factoring},
-that can mean to isolate the \emph{factors} of something. Here a \emph{factor}
+which can mean to isolate the \emph{factors} of something. Here a \emph{factor}
would be close to the mathematical definition of something that divides a
quantity, without leaving a remainder. Fowler is mixing the \emph{motivation}
behind refactoring into his definition. Instead it could be more refined, formed
to only consider the \emph{mechanical} and \emph{behavioral} aspects of
-refactoring. That is to factor the program again, putting it together in a
+refactoring. That is to factor the program again, putting it together in a
different way than before, while preserving the behavior of the program. An
alternative definition could then be:
was first published in 1984 by the \name{Forth Interest Group}. Then it was
reprinted in 1994 with minor typographical corrections, before it was
transcribed into an electronic edition typeset in \LaTeX\ and published under a
-Creative Commons licence in
+Creative Commons license in
2004. The edition cited here is the 2004 edition, but the content should
essentially be as in 1984.}. The exact word is only printed one
place~\cite[p.~232]{brodie2004}, but the term \emph{factoring} is prominent in
-the book, that also contains a whole chapter dedicated to (re)factoring, and how
-to keep the (Forth) code clean and maintainable.
+the book, which also contains a whole chapter dedicated to (re)factoring, and
+how to keep the (Forth) code clean and maintainable.
\begin{quote}
\ldots good factoring technique is perhaps the most important skill for a
\subsection{Reasons for refactoring}
There are many reasons why people want to refactor their programs. They can for
instance do it to remove duplication, break up long methods or to introduce
-design patterns into their software systems. The shared trait for all these are
+design patterns into their software systems. The shared trait for all these is
that peoples' intentions are to make their programs \emph{better}, in some
sense. But what aspects of their programs are becoming improved?
like when humans learn to read. First they must learn how to recognize letters.
Then they can learn distinct words, and later read sequences of words that form
whole sentences. Eventually, most of them will be able to read whole books and
-briefly retell the important parts of its content. This suggest that the use of
+briefly retell the important parts of its content. This suggests that the use of
design patterns is a good idea when reasoning about computer programs. With
extensive use of design patterns when creating complex program structures, one
does not always have to read whole classes of code to comprehend how they
\begin{description}
\item[1992] William F. Opdyke submits his doctoral dissertation called
\tit{Refactoring Object-Oriented Frameworks}\citing{opdyke1992}. This work
- defines a set of refactorings, that are behavior preserving given that their
+ defines a set of refactorings that are behavior-preserving given that their
preconditions are met. The dissertation is focused on the automation of
refactorings.
\item[1999] Martin Fowler et al.: \tit{Refactoring: Improving the Design of
Regarding the \MoveMethod refactoring, the \name{Eclipse} and \name{IntelliJ}
IDEs do the job in very similar manners. In most situations they both do a
satisfying job by producing the expected outcome. But they do nothing to check
-that the result does not break the semantics of the program \see{correctness}.
+that the result does not break the semantics of the program
+\see{sec:correctness}.
The \name{NetBeans} IDE implements this refactoring in a somewhat
unsophisticated way. For starters, the refactoring's default destination for the
move, is the same class as the method already resides in, although it refuses to
of \type{X} by accessing them through \var{c.x}, where \var{c} is a parameter of
type \type{C} that is added the method \method{f} when it is moved. (This is
seldom the desired outcome of this refactoring, but ironically, this ``feature''
-keeps \name{NetBeans} from breaking the code in the example from \myref{correctness}.)
-If \var{c.x} for some reason is inaccessible to \type{X}, as in this case, the
-refactoring breaks the code, and it will not compile. \name{NetBeans} presents a
-preview of the refactoring outcome, but the preview does not catch it if the IDE
-is about break the program.
+keeps \name{NetBeans} from breaking the code in the example from
+\myref{sec:correctness}.) If \var{c.x} for some reason is inaccessible to
+\type{X}, as in this case, the refactoring breaks the code, and it will not
+compile. \name{NetBeans} presents a preview of the refactoring outcome, but the
+preview does not catch it if the IDE is about break the program.
The IDEs under investigation seem to have fairly good support for primitive
refactorings, but what about more complex ones, such as
\begin{quote}
Software entities (classes, modules, functions, etc.) should be open for
- extension, but closed for modification.\footnote{See
- \url{http://c2.com/cgi/wiki?OpenClosedPrinciple} or
- \url{https://en.wikipedia.org/wiki/Open/closed_principle}}
+ extension, but closed for modification.
\end{quote}
Maintainability is often thought of as the ability to be able to introduce new
\subsubsection{The impact on performance}
\begin{quote}
- Refactoring certainly will make software go more slowly\footnote{With todays
+ Refactoring certainly will make software go more slowly\footnote{With today's
compiler optimization techniques and performance tuning of e.g. the Java
virtual machine, the penalties of object creation and method calls are
debatable.}, but it also makes the software more amenable to performance
Then it is crucial to have proper tool support that can perform them
automatically. Tools that can parse source code and thus have semantic knowledge
about which occurrences of which names belong to what construct in the program.
-For even trying to perform one of these complex task manually, one would have to
-be very confident on the existing test suite \see{testing}.
+For even trying to perform one of these complex tasks manually, one would have
+to be very confident on the existing test suite \see{testing}.
-\subsection{Correctness of refactorings}\label{correctness}
+\subsection{Correctness of refactorings}\label{sec:correctness}
For automated refactorings to be truly useful, they must show a high degree of
behavior preservation. This last sentence might seem obvious, but there are
examples of refactorings in existing tools that break programs. In an ideal
world, every automated refactoring would be ``complete'', in the sense that it
would never break a program. In an ideal world, every program would also be free
from bugs. In modern IDEs the implemented automated refactorings are working for
-\emph{most} cases, that is enough for making them useful.
+\emph{most} cases, which is enough for making them useful.
I will now present an example of a \emph{corner case} where a program breaks
when a refactoring is applied. The example shows an \ExtractMethod refactoring
particular situation without altering the program's behavior, mainly because
its \refa{Move Method} refactoring implementation is a bit flawed in other ways
\see{toolSupport}.}. The target and the destination for the composed
- refactoring is shown in \myref{lst:correctnessExtractAndMove}. Note that the
+ refactoring are shown in \myref{lst:correctnessExtractAndMove}. Note that the
method \method{m(C c)} of class \type{X} assigns to the field \var{x} of the
argument \var{c} that has type \type{C}.
\end{listing}
The bug introduced in the previous example is of such a nature\footnote{Caused
- by aliasing. See \url{https://en.wikipedia.org/wiki/Aliasing_(computing)}}
- that it is very difficult to spot if the refactored code is not covered by
- tests. It does not generate compilation errors, and will thus only result in
- a runtime error or corrupted data, which might be hard to detect.
+by aliasing.} that it is very difficult to spot if the refactored code is not
+covered by tests. It does not generate compilation errors, and will thus only
+result in a runtime error or corrupted data, which might be hard to detect.
\subsection{Refactoring and the importance of testing}\label{testing}
\begin{quote}
\end{quote}
When refactoring, there are roughly three classes of errors that can be made.
-The first class of errors are the ones that make the code unable to compile.
+The first class of errors is the one that makes the code unable to compile.
These \emph{compile-time} errors are of the nicer kind. They flash up at the
moment they are made (at least when using an IDE), and are usually easy to fix.
-The second class are the \emph{runtime} errors. Although they take a bit longer
-to surface, they usually manifest after some time in an illegal argument
+The second class is the \emph{runtime} errors. Although these errors take a bit
+longer to surface, they usually manifest after some time in an illegal argument
exception, null pointer exception or similar during the program execution.
-These kind of errors are a bit harder to handle, but at least they will show,
+These kinds of errors are a bit harder to handle, but at least they will show,
eventually. Then there are the \emph{behavior-changing} errors. These errors are
of the worst kind. They do not show up during compilation and they do not turn
on a blinking red light during runtime either. The program can seem to work
automated tests. Manual testing may have its uses, but when refactoring, it is
automated unit testing that dominate. For discovering behavior changes it is
especially important to have tests that cover potential problems, since these
-kind of errors does not reveal themselves.
+kinds of errors do not reveal themselves.
Unit testing is not a way to \emph{prove} that a program is correct, but it is a
way to make you confident that it \emph{probably} works as desired. In the
If the test coverage for a code base is perfect, then it should, theoretically,
be risk-free to perform refactorings on it. This is why automated tests and
-refactoring are such a great match.
+refactoring is such a great match.
\subsubsection{Testing the code from correctness section}
The worst thing that can happen when refactoring is to introduce changes to the
-behavior of a program, as in the example on \myref{correctness}. This example
-may be trivial, but the essence is clear. The only problem with the example is
-that it is not clear how to create automated tests for it, without changing it
-in intrusive ways.
+behavior of a program, as in the example on \myref{sec:correctness}. This
+example may be trivial, but the essence is clear. The only problem with the
+example is that it is not clear how to create automated tests for it, without
+changing it in intrusive ways.
Unit tests, as they are known from the different \glosspl{xUnit} around, are
only suitable to test the \emph{result} of isolated operations. They can not
\end{comment}
-\section{The Project}\label{sec:project}
+\section{The project}\label{sec:project}
In this section we look at the work that shall be done for this project, its
building stones and some of the methodologies used.
This composed refactoring is not well described in the literature, but it is
implemented in at least one tool called
\name{CodeRush}\footnote{\url{https://help.devexpress.com/\#CodeRush/CustomDocument3519}},
-that is an extension for \name{MS Visual
+which is an extension for \name{MS Visual
Studio}\footnote{\url{http://www.visualstudio.com/}}. In CodeRush it is called
\refa{Extract Method to
Type}\footnote{\url{https://help.devexpress.com/\#CodeRush/CustomDocument6710}},
There is a certain class of tools that meet these criteria, namely the class of
\emph{IDEs}\footnote{\emph{Integrated Development Environment}}. These are
-programs that is meant to support the whole production cycle of a computer
+programs that are meant to support the whole production cycle of a computer
program, and the most popular IDEs that support Java, generally have quite good
refactoring support.
IDE}. This is the most popular\citing{javaReport2011} among them and seems to be
de facto standard IDE for Java development regardless of platform.
+
\subsection{The primitive refactorings}
The refactorings presented here are the primitive refactorings used in this
project. They are the abstract building blocks used by the \ExtractAndMoveMethod
\label{lst:extractAndMoveMethodRefactoring}
\end{listing}
+\subsection{The Coupling Between Object Classes metric}\label{sec:CBO}
+The best known metric for measuring coupling between classes in object-oriented
+software is called \metr{Coupling Between Object Classes}, usually abbreviated
+as CBO. The metric is defined in the article \tit{A Metrics Suite for Object
+Oriented Design}\citing{metricsSuite1994} by Chidamber and Kemerer, published in
+1994.
+
+\definition{\emph{CBO} for a class is a count of the number of other classes to
+which it is coupled.}
+
+An object is coupled to another object if one of them acts on the other by using
+methods or instance variables of the other object. This relation goes both ways,
+so both outgoing and incoming uses are counted. Each coupling relationship is
+only considered once when measuring CBO for a class.
+
+\paragraph{How can the Extract and Move Method refactoring improve CBO?}
+\Myref{lst:CBOExample} shows how CBO changes for a class when it is refactored
+with the \ExtractAndMoveMethod refactoring. In the example we consider only the
+CBO value of class \type{C}.
+
+\begin{listing}[h]
+\begin{multicols}{2}
+\begin{minted}[linenos,samepage,frame=topline,label={Before},framesep=\mintedframesep]{java}
+class C {
+ A a; B b;
+ X x;
+ void method() {
+ x.y.foo();
+ x.y.bar();
+ }
+ /* Uses of A and B.
+ No uses of other
+ classes. */
+}
+
+class X {
+ Y y;
+ /* No uses of C.
+ Uses of Y. */
+}
+
+class Y {
+ void foo(){
+ /* No uses of C. */
+ }
+ void bar(){
+ /* No uses of C. */
+ }
+}
+\end{minted}
+
+\columnbreak
+
+\begin{minted}[linenos,samepage,frame=topline,label={After},framesep=\mintedframesep]{java}
+class C {
+ A a; B b;
+ X x;
+ void method() {
+ x.fooBar();
+ }
+ /* Uses of A and B.
+ No uses of other
+ classes. */
+}
+
+class X {
+ Y y;
+ /* No uses of C.
+ Uses of Y. */
+ void fooBar() {
+ y.foo();
+ y.bar();
+ }
+}
+
+class Y {
+ void foo(){
+ /* No uses of C. */
+ }
+ void bar(){
+ /* No uses of C. */
+ }
+}
+ \end{minted}
+\end{multicols}
+\caption{An example of improving CBO. Class \type{C} has a CBO value of 4
+before refactoring it, and 3 after.}
+\label{lst:CBOExample}
+\end{listing}
+
+Before refactoring the class \type{C} with the \ExtractAndMoveMethod
+refactoring, it has a CBO value of 4. The class uses members of the classes
+\type{A} and \type{B}, which accounts for 2 of the coupling relationships of
+class \type{C}. In addition to this, it uses its variable \var{x} with type
+\type{X} and also the methods \method{foo} and \method{bar} declared in class
+\type{Y}, giving it a total CBO value of 4.
+
+The after-part of the example code in \mysimpleref{lst:CBOExample} shows the
+result of extracting the lines
+5 and 6 of class \type{C} into a new method \method{fooBar}, with a subsequent
+ move of it to class \type{X}.
+
+With respect to the CBO metric, the refactoring action accomplishes something
+important: It eliminates the uses of class \type{Y} from class \type{C}. This
+means that the class \type{C} is no longer coupled to \type{Y}, only the classes
+\type{A}, \type{B} and \type{X}. The CBO value of class \type{C} is therefore 3
+after refactoring, while no other class have received any increase in CBO.
+
+The example shown here is an ideal situation. Coupling is reduced for one class
+without any increase of coupling for another class. There is also another point
+that is important. It is the fact that to reduce the CBO value for a class, we
+need to remove \emph{all} its uses of another class. This is done for the class
+\type{C} in \myref{lst:CBOExample}, where all uses of class \type{Y} is removed
+by the \ExtractAndMoveMethod refactoring.
+\todoin{Highlight code}
+
+
\subsection{Research questions}\label{sec:researchQuestions}
The main question that I seek an answer to in this thesis is:
changes that do not compile, or make changes that alter the behavior of the
program.
-\paragraph{Can we improve the quality of code?} Assuming that the refactoring is
-safe: Is it feasible to assure that the code we refactor actually gets better in
-terms of coupling?
+\paragraph{Can we improve the quality of source code?} Assuming that the
+refactoring is safe: Is it feasible to assure that the code we refactor actually
+gets better in terms of coupling?
\paragraph{How can the automation of the refactoring be helpful?} Assuming the
refactoring does in fact improve the quality of source code and is safe to use:
what parts of the development process can the refactoring play a role?
\subsection{Methodology}
+This section will present some of the methods used during the work of this
+thesis.
\subsubsection{Evolutionary design}
-In the programming work for this project, it have tried to use a design strategy
+In the programming work for this project, I have tried using a design strategy
called evolutionary design, also known as continuous or incremental
-design\citing{wiki_continuous_2014}. It is a software design strategy
-advocated by the Extreme Programming community. The essence of the strategy is
-that you should let the design of your program evolve naturally as your
-requirements change. This is seen in contrast with up-front design, where
-design decisions are made early in the process.
+design\citing{wiki_continuous_2014}. It is a software design strategy advocated
+by the Extreme Programming community. The essence of the strategy is that you
+should let the design of your program evolve naturally as your requirements
+change. This is seen in contrast with up-front design, where design decisions
+are made early in the process.
The motivation behind evolutionary design is to keep the design of software as
simple as possible. This means not introducing unneeded functionality into a
by the tests. However, this does not prove the correctness of the analysis
leading to the test definitions.
-\subsection{Dogfooding}
-\todoin{Write}
+\subsection{Case studies}
+The case study methodology is used to show how the \ExtractAndMoveMethod
+refactoring performs on real code, not just toy examples. The case studies are
+used to analyze our project so we can conclude on its completeness and
+usefulness.
-\section{Related Work}\label{sec:relatedWork}
+\subsection{Dogfooding}
+Dogfooding is a methodology where you use your own tools to do your job, also
+referred to as ``eating your own dog food''\citing{harrisonDogfooding2006}. It
+is used in this project to see if we can refactor our own refactoring code and
+still use it to refactor other code.
+
+\section{Related work}\label{sec:relatedWork}
+Here we present some work related to automated composition of refactorings.
+
+\subsection{Refactoring safety}\label{sec:saferRefactoring}
+This section presents a couple of approaches to improving the safety of
+performing refactorings. In these approaches, the problems that are addressed
+are not compilation problems, but behavior-altering problems that are not easily
+discovered during static analysis of source code. An example of such a problem
+is presented in \myref{sec:correctness}.
+
+\subsubsection{Project ``Safer Refactorings''}
+\tit{Safer Refactorings}\citing{stolzSaferRefactorings} is a proposal for a
+master's thesis. The proposer is my supervisor, Volker Stolz from the University
+of Oslo.
+
+The proposed solution for making refactorings safer, is to insert assertions
+into source code when refactoring it. For the example in
+\myref{lst:correctnessExtractAndMoveResult}, which is the result of a
+refactoring, it is suggested that we insert an assert statement between lines 9
+and 10. In this example, the assert statement
+would be \mint{java}|assert c.x == this;| which would discover the aliasing
+problems of this example.
+
+\subsubsection{``Making Program Refactoring Safer''}
+This is the name of an article\citing{soaresSafer2010} about providing a way to
+improve safety during refactoring. Soares et al. approaches the problem of
+preserving behavior during refactoring by analyzing a transformation and then
+generate a test suite for it, using static analysis. These tests are then run
+for both the before- and after-code, and is compared to assure that they are
+consistent.
+
+\subsection{Search-based refactoring}
+\tit{Search-Based Refactoring: an
+empirical study}\citing{okeeffeSearchBased2008} is a paper by Mark O'Keeffe and
+Mel Ó Cinnéide published in 2008. The authors present an empirical study of
+different algorithmic approaches to search-based refactoring.
+
+The common approach for all these algorithms is to generate a set of changes to
+a program for then to use a ``fitness function'' to evaluate if they improve its
+design or not. The fitness function consists of a weighted sum of different
+object-oriented metrics.
+
+Among other things, the authors conclude that even with small input programs,
+their solution representation is memory-intensive, at least for some algorithms.
+The programs they refactor on have in average 4,000 lines of code, spread over
+57 classes. I.e. considerably smaller than one of the programs that will be
+ subject to refactoring in this project.
-\subsection{Safer refactorings}
-\todoin{write}
\subsection{The compositional paradigm of refactoring}
This paradigm builds upon the observation of Vakilian et
Vakilian and his associates have performed a survey of the effectiveness of the
compositional paradigm versus the wizard-based one. They claim to have found
evidence of that the \emph{compositional paradigm} outperforms the
-\emph{wizard-based}. It does so by reducing automation, which seem
+\emph{wizard-based}. It does so by reducing automation, which seems
counterintuitive. Therefore they ask the question ``What is an appropriate level
of automation?'', and thus questions what they feel is a rush toward more
automation in the software engineering community.
specific portion of text we are about to deal with.
To be able to clearly reason about a text selection done to a portion of text in
-a computer file, that consist of pure text, we put up the following definition.
+a computer file, which consists of pure text, we put up the following
+definition:
\definition{A \emph{text selection} in a text file is defined by two
non-negative integers, in addition to a reference to the file itself. The first
\section{Where we look for text selections}
\subsection{Text selections are found in methods}
-The text selections we are interested in are those that surrounds program
+The text selections we are interested in are those that surround program
statements. Therefore, the place we look for selections that can form candidates
for an execution of the \ExtractAndMoveMethod refactoring, is within the body of
a single method.
\ExtractAndMoveMethod refactoring. The reason for this is that in the cases
where we want to perform the refactoring for a selection within a static method,
the first step is to extract the selection into a new method. Hence this method
-also become static, since it must be possible to call it from a static context.
+also becomes static, since it must be possible to call it from a static context.
It would then be difficult to move the method to another class, make it
non-static and calling it through a variable. To avoid these obstacles, we
simply ignore static methods.
\end{listing}
\subsection{The possible text selections of a method body}
-\todoin{dummy todo}
The number of possible text selections that can be made from the text in a
method body, are equal to all the sub-sequences of characters within it. For our
purposes, analyzing program source code, we must define what it means for a text
\end{listing}
Each nesting level of a method body can have many such sequences of statements.
-The outermost nesting level has one such sequence, and each branch contains
-their own sequence of statements. \Myref{lst:grandExample} has a version of some
+The outermost nesting level has one such sequence, and each branch contains
+its own sequence of statements. \Myref{lst:grandExample} has a version of some
code where all such sequences of statements are highlighted for a method body.
To complete our example of possible text selections, I will now list all
\end{description}
\subsubsection{The complexity}\label{sec:complexity}
-The complexity of how many text selections that needs to be analyzed for a body
+The complexity of how many text selections that need to be analyzed for a body
of in total $n$ statements, is bounded by $O(n^2)$. A body of statements is here
all the statements in all nesting levels of a sequence of statements. A method
body (or a block) is a body of statements. To prove that the complexity is
-bounded by $O(n^2)$, I present a couple of theorems and proves them.
+bounded by $O(n^2)$, I present a couple of theorems and prove them.
\begin{theorem}
The number of text selections that need to be analyzed for each list of
\end{proof}
-Therefore, the complexity for the number of selections that needs to be analyzed
+Therefore, the complexity for the number of selections that need to be analyzed
for a body of $n$ statements is $O\bigl(\frac{n(n+1)}{2}\bigr) = O(n^2)$.
\section{Disqualifying a selection}
\ExtractAndMoveMethod refactoring. To avoid this, we have to check all text
selections for such conditions before they are further analyzed. This section
is therefore going to present some properties that make a selection unsuitable
-for our refactoring.
+for our refactoring. When analyzing all these properties, it is assumed that the
+source code does not contain any compilation errors.
\subsection{A call to a protected or package-private method}
If a text selection contains a call to a protected or package-private method, it
Situations like this would lead to code that will not compile. Therefore, we
have to avoid them by not allowing selections to contain such double class
-instance creations that also contains references to fields.
+instance creations that also contain references to fields.
\begin{comment}
\todoin{File Eclipse bug report}
\end{comment}
Performing a move on a method that instantiates a non-static inner class, will
break the code if the instantiation is not handled properly. For this reason,
-selections that contains instantiations of non-static inner classes are deemed
+selections that contain instantiations of non-static inner classes are deemed
unsuitable for the \ExtractAndMoveMethod refactoring.
\subsection{References to enclosing instances of the enclosing class}
-The title of this section may be a little hard to grasp at first. What it means
-is that there is a (non-static) class \m{C} that is declared in the scope of
-possibly multiple other classes. And there is a statement in the body of a
-method declared in class \m{C}, that contains a reference to one or more
+To ``reference an enclosing instance of the enclosing class'' is to reference
+another instance than the one for the immediately enclosing class. Imagine there
+is a (non-static) class \m{C} that is declared in the inner scope of another
+class. That class can again be nested inside a third class, and so on. Hence,
+the nested class \m{C} can have access to many enclosing instances of its
+innermost enclosing class. A selection in a method declared in class \m{C} is
+disqualified if it contains a statement that contains a reference to one or more
instances of these enclosing classes of \m{C}.
The problem with this, is that these references may not be valid if they are
return type, then a call to it would also be a valid return point for the
calling method. If its return value is of the void type, then the \ExtractMethod
refactoring will append an empty return statement to the back of the method
-call. Therefore, the analysis does not discriminate on either kinds of return
+call. Therefore, the analysis does not discriminate on either kind of return
statements, with or without a return value.
-A throw statement is accepted anywhere a return statement is required. This is
-because a throw statement causes an immediate exit from the current block,
-together with all outer blocks in its control flow that does not catch the
-thrown exception.
-
-Return statements can be either explicit or implicit. An \emph{explicit} return
-statement is formed by using the \code{return} keyword, while an \emph{implicit}
-return statement is a statement that is not formed using \code{return}, but must
-be the last statement of a method that can have any side effects. This can
-happen in methods with a void return type. An example is a statement that is
-inside one or more blocks. The last statement of a method could for instance be
-a synchronized statement, but the last statement that is executed in the method,
-and that can have any side effects, may be located inside the body of the
-synchronized statement.
+A \emph{throw} statement is accepted anywhere a return statement is required.
+This is because a throw statement causes an immediate exit from the current
+block, together with all outer blocks in its control flow that does not catch
+the thrown exception.
+
+We separate between explicit and implicit return statements. An \emph{explicit}
+return statement is formed by using the \code{return} keyword, while an
+\emph{implicit} return statement is a statement that is not formed using
+\code{return}, but must be the last statement of a method that can have any side
+effects. This can happen in methods with a void return type. An example is a
+statement that is inside one or more blocks. The last statement of a method
+could for instance be a synchronized statement, but the last statement that is
+executed in the method, and that can have any side effects, may be located
+inside the body of the synchronized statement.
We can start the check for this property by looking at the last statement of a
selection to see if it is a return statement (explicit or implicit) or a throw
statement. If this is the case, then the property holds, assuming the selected
-code does not contain any compilation errors. All execution paths within the
+code do not contain any compilation errors. All execution paths within the
selection should end in either this, or another, return or throw statement.
-\todoin{State somewhere that we assume no compilation errors?}
-
-If the last statement of the selection is not a return or throw, the execution
-of it must eventually end in one for the selection to be legal. This means that
-all branches of the last statement of every branch must end in a return or
-throw. Given this recursive definition, there are only five types of statements
-that are guaranteed to end in a return or throw if their child branches does.
-All other statements would have to be considered illegal. The first three:
-Block-statements, labeled statements and do-statements are all kinds of
-fall-through statements that always gets their body executed. Do-statements
-would not make much sense if written such that they
-always ends after the first round of execution of their body, but that is not
-our concern. The remaining two statements that can end in a return or throw are
+
+If the last statement of the selection is not a \emph{return} or \emph{throw},
+the execution of it must eventually end in one of these types of statements for
+the selection to be legal. This means that all branches of the last statement of
+every branch must end in a return or throw. Given this recursive definition,
+there are only five types of statements that are guaranteed to end in a return
+or throw if their child branches do. All other statements would have to be
+considered illegal. The first three: Block-statements, labeled statements and
+do-statements are all kinds of fall-through statements that always get their
+body executed. Do-statements would not make much sense if written such that they
+always end after the first round of execution of their body, but that is not our
+concern. The remaining two statements that can end in a return or throw are
if-statements and try-statements.
For an if-statement, the rule is that if its then-part does not contain any
Try-statements are handled much the same way as if-statements. The body of a
try-statement must contain a return or throw. The same applies to its catch
-clauses and finally body.
+clauses and finally body. \todoin{finally body?}
\subsection{Ambiguous return values}
-The problem with ambiguous return values arise when a selection is chosen to be
-extracted into a new method, but it needs to return more than one value from
-that method.
+The problem with ambiguous return values arises when a selection is chosen to be
+extracted into a new method, but if refactored it needs to return more than one
+value from that method.
-This problem occurs in two situations. The first situation arise when there is
+This problem occurs in two situations. The first situation arises when there is
more than one local variable that is both assigned to within a selection and
-also referenced after the selection. The other situation occur when there is
+also referenced after the selection. The other situation occurs when there is
only one such assignment, but the selection also contain return statements.
Therefore we must examine the selection for assignments to local variables that
to non-final variables and assignments to array access. All other assignments
are regarded illegal.
-\todoin{Expand with more illegal statements and/or conclude that I did not have
-time to analyze all statement types.}
+\paragraph{Incompleteness.} The list of illegal statements is not complete, and
+a lot of situations that can lead to compilation errors or behavior changes are
+not considered. It is not feasible to consider all such situations within the
+limits of this master's project, and maybe not outside of them either. The
+feasibility of this problem could be explored further by others.
\section{Disqualifying selections from the
example}\label{sec:disqualifyingExample}
\section{Finding a move target}
In the analysis needed to perform the \ExtractAndMoveMethod refactoring
automatically, the selection we choose is found among all the selections that
-has a possible move target. Therefore, the best possible move target must be
+have a possible move target. Therefore, the best possible move target must be
found for all the candidate selections, so that we are able to sort out the
selection that is best suited for the refactoring.
The names we are looking for, we call prefixes. This is because we are not
interested in names that occur in the middle of a dot-separated sequence of
-names. We are only interested in names that constitutes prefixes of other names,
+names. We are only interested in names constituting prefixes of other names, and
possibly themselves. The reason for this, is that two lexically equal names need
not be referencing the same variable, if they themselves are not referenced via
the same prefix. Consider the two method calls \code{a.x.foo()} and
referenced within the selection.
\section{Unfixes}\label{s:unfixes}
-The prefixes that are not valid as move targets are called unfixes.
+We will call the prefixes that are not valid as move targets for unfixes.
-An unfix can be a name that is assigned to within a selection. The reason that
-this cannot be allowed, is that the result would be an assignment to the
-\type{this} keyword, which is not valid in Java \see{eclipse_bug_420726}.
+A name that is assigned to within a selection can be an unfix. The reason for
+this is that the result would be an assignment to the \type{this} keyword, which
+is not valid in Java \see{eclipse_bug_420726}.
-Prefixes that originates from variable declarations within the same selection
-are also considered unfixes. This is because when a method is moved, it needs to
-be called through a variable. If this variable is also declared within the
-method that is to be moved, this obviously cannot be done.
+Prefixes that originate from variable declarations within the same selection are
+also considered unfixes. The reason for this is that when a method is moved, it
+needs to be called through a variable. If this variable is also declared within
+the method that is to be moved, this obviously cannot be done.
Also considered as unfixes are variable references that are of types that are
not suitable for moving methods to. This can either be because it is not
physically possible to move a method to the desired class or that it will cause
compilation errors by doing so.
-If the type binding for a name is not resolved it is considered and unfix. The
-same applies to types that is only found in compiled code, so they have no
+If the type binding for a name is not resolved it is considered an unfix. The
+same applies to types that are only found in compiled code, so they have no
underlying source that is accessible to us. (E.g. the \type{java.lang.String}
class.)
-Interfaces types are not suitable as targets. This is simply because interfaces
+Interface types are not suitable as targets. This is simply because interfaces
in Java cannot contain methods with bodies. (This thesis does not deal with
features of Java versions later than Java 7. Java 8 has interfaces with default
implementations of methods.)
parameter. This new method would need to live in the scope of the declaring
class of the originating method. The local class would then not be in the scope
of the extracted method, thus bringing the source code into an illegal state.
-One could imagine that the method was extracted and moved in one operation,
-without an intermediate state. Then it would make sense to include variables
-with types of local classes in the set of legal targets, since the local classes
-would then be in the scopes of the method calls. If this makes any difference
-for software metrics that measure coupling would be a different discussion.
+This scenario is shown in \myref{lst:extractMethodLocalClass}. One could imagine
+that the method was extracted and moved in one operation, without an
+intermediate state. Then it would make sense to include variables with types of
+local classes in the set of legal targets, since the local classes would then be
+in the scopes of the method calls. If this makes any difference for software
+metrics that measure coupling would be a different discussion.
+\todoin{highlight code!}
\begin{listing}[htb]
\begin{multicols}{2}
fooBar(inst);
}
-// Intermediate step
+// Illegal intermediate step
void fooBar(LocalClass inst) {
inst.foo();
inst.bar();
}
\end{minted}
\end{multicols}
-\caption{When the \refa{Extract and Move Method} tries to use a variable with a
-local type as the move target, an intermediate step is performed that is not
-allowed. Here: \type{LocalClass} is not in the scope of \method{fooBar} in its
-intermediate location.}
-\label{lst:extractMethod_LocalClass}
+\caption{The \refa{Extract and Move Method} refactoring bringing the code into
+an illegal state with an intermediate step.}
+\label{lst:extractMethodLocalClass}
\end{listing}
The last class of names that are considered unfixes are names used in null
-tests. These are tests that reads like this: if \code{<name>} equals \var{null}
+tests. These are tests that read like this: if \code{<name>} equals \var{null}
then do something. If allowing variables used in those kinds of expressions as
targets for moving methods, we would end up with code containing boolean
-expressions like \code{this == null}, which would not be meaningful, since
-\var{this} would never be \var{null}.
+expressions like \code{this == null}, which would always evaluate to
+\code{false}, since \var{this} would never be \var{null}. The existence of a
+null test indicates that a variable is expected to sometimes hold the value
+\var{null}. By using a variable used in a null test as a move target, we could
+potentially end up with a
+null pointer exception if the method is called on a variable with a null value.
\section{Finding the example selections that have possible targets}
We now pick up the thread from \myref{sec:disqualifyingExample} where we have a
-set of text selections that needs to be analyzed to find out if some of them are
+set of text selections that need to be analyzed to find out if some of them are
suitable targets for the \ExtractAndMoveMethod refactoring.
We start by analyzing the text selections for nesting level 2, because these
\item[Selection $(5,9)$.] \hfill \\
This selection contains two variable references that must be analyzed to see
if they are possible move candidates. The first one is the variable
- \var{bool}. This variable is of type \type{boolean}, that is a primary type
+ \var{bool}. This variable is of type \type{boolean}, which is a primary type
and therefore not possible to make any changes to. The second variable is
\var{a}. The variable \var{a} is an unfix in $(5,9)$, for the same reason as
in the selections $(6,7)$, $(7,8)$ and $(6,8)$. So selection $(5,9)$
only found in one statement, and in a prefix with only one segment, so it is
not considered to be a possible move target. The only variable left is
\var{b}. Just as in the selection $(16,17)$, \var{b} is part of the prefix
- \code{b.a}, that has 2 appearances. We have found a new candidate $((14,21),
- \texttt{b.a}, f(2))$.
+ \code{b.a}, which has 2 appearances. We have found a new candidate
+ $((14,21), \texttt{b.a}, f(2))$.
\end{description}
selections are considered to be equally good candidates for the
\ExtractAndMoveMethod refactoring.
+\section{Performing changes}
+When a text selection and a move target is found for the \ExtractAndMoveMethod
+refactoring, the actual changes are executed by two existing primitive
+refactorings. First the \ExtractMethod refactoring is used to extract the
+selection into a new method. Then the \MoveMethod refactoring is used to move
+that new method to the class determined by the move target.
+
+If, at any point, an exception is thrown or the preconditions for one of the
+primitive refactorings are not satisfied, the composite refactoring is aborted,
+and the source code is left in its current state. This has the implication that
+the \ExtractAndMoveMethod refactoring could end up being partially executed.
+This happens if the \ExtractMethod refactoring is executed, but the \MoveMethod
+refactoring is being canceled. A partial execution is not considered a problem,
+since the code should still compile.
+
+\todoin{Pointing to implementation chapter}
+
\section{Concluding the example}
For choosing one of the remaining selections, we need to order our candidates
after the criteria in the previous section. Below we have the candidates ordered
\label{lst:grandExampleResult}
\end{listing}
+
\chapter{Refactorings in Eclipse JDT: Design and
-Shortcomings}\label{ch:jdt_refactorings}
+shortcomings}\label{ch:jdt_refactorings}
This chapter will deal with some of the design behind refactoring support in
\name{Eclipse}, and the JDT in specific. After which it will follow a section
\subsubsection{The Refactoring Class}
The abstract class \type{Refactoring} is the core of the LTK framework. Every
-refactoring that is going to be supported by the LTK have to end up creating an
+refactoring that is going to be supported by the LTK has to end up creating an
instance of one of its subclasses. The main responsibilities of subclasses of
-\type{Refactoring} is to implement template methods for condition checking
+\type{Refactoring} are to implement template methods for condition checking
(\methodwithref{org.eclipse.ltk.core.refactoring.Refactoring}{checkInitialConditions}
and
\methodwithref{org.eclipse.ltk.core.refactoring.Refactoring}{checkFinalConditions}),
It then delegates to its given
\typewithref{org.eclipse.ltk.core.refactoring.participants}{RefactoringProcessor}
for condition checking and change creation. Participating in a refactoring can
-be useful in cases where the changes done to programming source code affects
+be useful in cases where the changes done to programming source code affect
other related resources in the workspace. This can be names or paths in
configuration files, or maybe one would like to perform additional logging of
changes done in the workspace.
\subsubsection{The Change Class}
This class is the base class for objects that is responsible for performing the
actual workspace transformations in a refactoring. The main responsibilities for
-its subclasses is to implement the
+its subclasses are to implement the
\methodwithref{org.eclipse.ltk.core.refactoring.Change}{perform} and
\methodwithref{org.eclipse.ltk.core.refactoring.Change}{isValid} methods. The
\method{isValid} method verifies that the change object is valid and thus can be
I will begin at the end and work my way toward the composition part of this
section.
-\subsection{Absence of Generics in Eclipse Source Code}
+\subsection{Absence of generics in Eclipse source code}
This section is not only concerning the JDT refactoring API, but also large
quantities of the \name{Eclipse} source code. The code shows a striking absence of the
Java language feature of generics. It is hard to read a class' interface when
addition, it results in a lot of ugly code, making the use of typecasting more
of a rule than an exception.
-\subsection{Composite Refactorings Will Not Appear as Atomic Actions}
+\subsection{Composite refactorings will not appear as atomic actions}
+When composing primitive refactorings from the JDT, it is not possible to make
+them appear as being executed as one change, but only as multiple small changes.
\subsubsection{Missing Flexibility from JDT Refactorings}
The JDT refactorings are not made with composition of refactorings in mind. When
-\chapter{Composite Refactorings in Eclipse}
+\chapter{Composite refactorings in Eclipse}
-\section{A Simple Ad Hoc Model}
+\section{A simple ad hoc model}
As pointed out in \myref{ch:jdt_refactorings}, the \name{Eclipse} JDT refactoring model
is not very well suited for making composite refactorings. Therefore a simple
model using changer objects (of type \type{RefaktorChanger}) is used as an
preconditions validate, the upcoming changes are executed by an
\typewithref{no.uio.ifi.refaktor.change.executors}{Executor}.
-\section{The Extract and Move Method Refactoring}
+\section{The Extract and Move Method refactoring}
%The Extract and Move Method Refactoring is implemented mainly using these
%classes:
%\begin{itemize}
% \item \type{PrefixSet}
%\end{itemize}
-\subsection{The Building Blocks}
+\subsection{The building blocks}
This is a composite refactoring, and hence is built up using several primitive
refactorings. These basic building blocks are, as its name implies, the
\ExtractMethod refactoring\citing{refactoring} and the \MoveMethod
where the last class is designed to be used together with the processor-based
\typewithref{org.eclipse.ltk.core.refactoring.participants}{MoveRefactoring}.
-\subsubsection{The ExtractMethodRefactoring Class}
+\subsubsection{The ExtractMethodRefactoring class}
This class is quite simple in its use. The only parameters it requires for
construction is a compilation
unit\typeref{org.eclipse.jdt.core.ICompilationUnit}, the offset into the source
extracted. Then you have to set the method name for the new method together with
its visibility and some not so interesting parameters.
-\subsubsection{The MoveInstanceMethodProcessor Class}
+\subsubsection{The MoveInstanceMethodProcessor class}
For the \refa{Move Method}, the processor requires a little more advanced input than
the class for the \refa{Extract Method}. For construction it requires a method
-handle\typeref{org.eclipse.jdt.core.IMethod} for the method that is to be moved.
-Then the target for the move have to be supplied as the variable binding from a
-chosen variable declaration. In addition to this, one have to set some
-parameters regarding setters/getters, as well as delegation.
+handle\typeref{org.eclipse.jdt.core.IMethod} for the method that is to be moved.
+Then the target for the move has to be supplied as the variable binding from a
+chosen variable declaration. In addition to this, some parameters have to be set
+regarding setters/getters, as well as delegation.
-To make a working refactoring from the processor, one have to create a
-\type{MoveRefactoring} with it.
+To make the processor a working refactoring, a \type{MoveRefactoring} must be
+created with it as a parameter.
-\subsection{The ExtractAndMoveMethodChanger}
+\subsection{The ExtractAndMoveMethodChanger class}
The \typewithref{no.uio.ifi.refaktor.changers}{ExtractAndMoveMethodChanger}
class is a subclass of the class
For finding the best suitable target the analyzer is using a
\typewithref{no.uio.ifi.refaktor.analyze.collectors}{PrefixesCollector} that
collects all the possible candidate targets for the refactoring. All the
-non-candidates is found by an
+non-candidates are found by an
\typewithref{no.uio.ifi.refaktor.analyze.collectors}{UnfixesCollector} that
collects all the targets that will give some kind of error if used. (For
details about the property collectors, see \myref{propertyCollectors}.) All
prefixes (and unfixes) are represented by a
\typewithref{no.uio.ifi.refaktor.extractors}{Prefix}, and they are collected
-into sets of prefixes. The safe prefixes is found by subtracting from the set of
-candidate prefixes the prefixes that is enclosing any of the unfixes. A prefix
-is enclosing an unfix if the unfix is in the set of its sub-prefixes. As an
-example, \code{``a.b''} is enclosing \code{``a''}, as is \code{``a''}. The safe
-prefixes is unified in a \type{PrefixSet}. If a prefix has only one occurrence,
-and is a simple expression, it is considered unsuitable as a move target. This
-occurs in statements such as \code{``a.foo()''}. For such statements it bares no
-meaning to extract and move them. It only generates an extra method and the
-calling of it.
+into sets of prefixes. The safe prefixes are found by subtracting from the set
+of candidate prefixes the prefixes that is enclosing any of the unfixes. A
+prefix is enclosing an unfix if the unfix is in the set of its sub-prefixes. As
+an example, \code{``a.b''} is enclosing \code{``a''}, as is \code{``a''}. The
+safe prefixes is unified in a \type{PrefixSet}. If a prefix has only one
+occurrence, and is a simple expression, it is considered unsuitable as a move
+target. This occurs in statements such as \code{``a.foo()''}. For such
+statements it bares no meaning to extract and move them. It only generates an
+extra method and the calling of it.
The most suitable target for the refactoring is found by finding the prefix with
the most occurrences. If two prefixes have the same occurrence count, but they
The handle for the method to be moved is found on the basis of the information
gathered after the execution of the \refa{Extract Method} refactoring. The only
information the \type{ExtractMethodRefactoring} is sharing after its execution,
-regarding find the method handle, is the textual representation of the new
+regarding finding the method handle, is the textual representation of the new
method signature. Therefore it must be parsed, the strings for types of the
parameters must be found and translated to a form that can be used to look up
-the method handle from its type handle. They have to be on the unresolved
-form.\todo{Elaborate?} The name for the type is found from the original
-selection, since an extracted method must end up in the same type as the
-originating method.
+the method handle from its type handle. They have to be on the unresolved form.
+The name for the type is found from the original selection, since an extracted
+method must end up in the same type as the originating method.
When analyzing a selection prior to performing the \refa{Extract Method} refactoring, a
target is chosen. It has to be a variable binding, so it is either a field or a
\subsubsection{The SearchBasedExtractAndMoveMethodAnalyzer}
This analyzer is responsible for analyzing all the possible text selections of a
-method and then choose the best result out of the analysis results that is, by
-the analyzer, considered to be the potential candidates for the Extract and Move
-Method refactoring.
+method and then to choose the best result out of the analysis results that are,
+by the analyzer, considered to be the potential candidates for the
+\ExtractAndMoveMethod refactoring.
Before the analyzer is able to work with the text selections of a method, it
needs to generate them. To do this, it parses the method to obtain a
\paragraph{Finding the candidate} for the refactoring is done by analyzing all
the generated text selection with the \type{ExtractAndMoveMethodAnalyzer}
\see{extractAndMoveMethodAnalyzer}. If the analyzer generates a useful result,
-an \type{ExtractAndMoveMethodCandidate} is created from it, that is kept in a
+an \type{ExtractAndMoveMethodCandidate} is created from it, which is kept in a
list of potential candidates. If no candidates are found, the
\type{NoTargetFoundException} is thrown.
ExtractAndMoveMethodCandidateComparator/FavorNoUnfixesCandidateComparator}
-\subsection{The Prefix Class}
+\subsection{The Prefix class}
This class exists mainly for holding data about a prefix, such as the expression
that the prefix represents and the occurrence count of the prefix within a
selection. In addition to this, it has some functionality such as calculating
intersection between two prefixes is a prefix representing the longest common
expression between the two.
-\subsection{The PrefixSet Class}
+\subsection{The PrefixSet class}
A prefix set holds elements of type \type{Prefix}. It is implemented with the
help of a \typewithref{java.util}{HashMap} and contains some typical set
operations, but it does not implement the \typewithref{java.util}{Set}
prefixes of another set. This is kind of a set difference operation only for
enclosing prefixes.
-\subsection{Hacking the Refactoring Undo
-History}\label{hacking_undo_history}
+\subsection{Hacking the refactoring undo
+history}\label{hacking_undo_history}
\todoin{Where to put this section?}
As an attempt to make multiple subsequent changes to the workspace appear as a
Unfortunately, not everything works as desired with this solution. The grouping
of the undo changes into the composite change does not make the undo operation
appear as an atomic operation. The undo operation is still split up into
-separate undo actions, corresponding to the change done by its originating
-refactoring. And in addition, the undo actions has to be performed separate in
+separate undo actions, corresponding to the changes done by their originating
+refactorings. And in addition, the undo actions have to be performed separate in
all the editors involved. This makes it no solution at all, but a step toward
something worse.
There might be a solution to this problem, but it remains to be found. The
-design of the refactoring undo management is partly to be blamed for this, as it
-it is to complex to be easily manipulated.
-
+design of the refactoring undo management is partly to be blamed for this, as
+it is too complex to be easily manipulated.
-
-\chapter{Analyzing Source Code in Eclipse}
+\chapter{Analyzing source code in Eclipse}
\section{The Java model}\label{javaModel}
The Java model of \name{Eclipse} is its internal representation of a Java project. It
is light-weight, and has only limited possibilities for manipulating source
code. It is typically used as a basis for the Package Explorer in \name{Eclipse}.
-The elements of the Java model is only handles to the underlying elements. This
+The elements of the Java model are only handles to the underlying elements. This
means that the underlying element of a handle does not need to actually exist.
Hence the user of a handle must always check that it exist by calling the
\method{exists} method of the handle.
-The handles with descriptions is listed in \myref{tab:javaModel}, while the
+The handles with descriptions are listed in \myref{tab:javaModel}, while the
hierarchy of the Java Model is shown in \myref{fig:javaModel}.
\begin{table}[htb]
\label{fig:javaModel}
\end{figure}
-\section{The Abstract Syntax Tree}
+\section{The abstract syntax tree}
\name{Eclipse} is following the common paradigm of using an abstract syntax tree for
source code analysis and manipulation.
bracket \var{\{} or a \var{nameToken}. It is recognized by the scanner on the
basis of something equivalent of a regular expression. This part of the process
is often implemented with the use of a finite automata. In fact, it is common to
-specify the tokens in regular expressions, that in turn is translated into a
+specify the tokens in regular expressions, which in turn are translated into a
finite automata lexer. This process can be automated.
The program component used to translate a stream of tokens into something
meaningful, is called a parser. A parser is fed tokens from the scanner and
performs an analysis of the structure of a program. It verifies that the syntax
-is correct according to the grammar rules of a language, that is usually
+is correct according to the grammar rules of a language, that are usually
specified in a context-free grammar, and often in a variant of the
-\name{Backus--Naur
-Form}\footnote{\url{https://en.wikipedia.org/wiki/Backus-Naur\_Form}}. The
-result coming from the parser is in the form of an \emph{Abstract Syntax Tree},
-AST for short. It is called \emph{abstract}, because the structure does not
-contain all of the tokens produced by the scanner. It only contain logical
-constructs, and because it forms a tree, all kinds of parentheses and brackets
-are implicit in the structure. It is this AST that is used when performing the
-semantic analysis of the code.
+\name{Backus--Naur Form}. The result coming from the parser is in the form of an
+\emph{Abstract Syntax Tree}, AST for short. It is called \emph{abstract},
+because the structure does not contain all of the tokens produced by the
+scanner. It only contains logical constructs, and because it forms a tree, all
+kinds of parentheses and brackets are implicit in the structure. It is this AST
+that is used when performing the semantic analysis of the code.
As an example we can think of the expression \code{(5 + 7) * 2}. The root of
this tree would in \name{Eclipse} be an \type{InfixExpression} with the operator
-\var{TIMES}, and a left operand that is also an \type{InfixExpression} with the
-operator \var{PLUS}. The left operand \type{InfixExpression}, has in turn a left
-operand of type \type{NumberLiteral} with the value \var{``5''} and a right
+\var{TIMES}, and a left operand, which is also an \type{InfixExpression} with
+the operator \var{PLUS}. The left operand \type{InfixExpression}, has in turn a
+left operand of type \type{NumberLiteral} with the value \var{``5''} and a right
operand \type{NumberLiteral} with the value \var{``7''}. The root will have a
right operand of type \type{NumberLiteral} and value \var{``2''}. The AST for
this expression is illustrated in \myref{fig:astInfixExpression}.
\caption{The format of the abstract syntax tree in \name{Eclipse}.}
\label{fig:astEclipse}
\end{figure}
-\todoin{Add more to the AST format tree? \myref{fig:astEclipse}}
\section{The ASTVisitor}\label{astVisitor}
So far, the only thing that has been addressed is how the data that is going to
be the basis for our analysis is structured. Another aspect of it is how we are
going to traverse the AST to gather the information we need, so we can conclude
-about the properties we are analysing. It is of course possible to start at the
+about the properties we are analyzing. It is of course possible to start at the
top of the tree, and manually search through its nodes for the ones we are
looking for, but that is a bit inconvenient. To be able to efficiently utilize
such an approach, we would need to make our own framework for traversing the
The use of the visitor pattern can be appropriate when the hierarchy of elements
is mostly stable, but the family of operations over its elements is constantly
-growing. This is clearly the case for the \name{Eclipse} AST, since the hierarchy of
-type \type{ASTNode} is very stable, but the functionality of its elements is
-extended every time someone needs to operate on the AST. Another aspect of the
-\name{Eclipse} implementation is that it is a public API, and the visitor pattern is an
-easy way to provide access to the nodes in the tree.
+growing. This is clearly the case for the \name{Eclipse} AST, since the
+hierarchy for the type \type{ASTNode} is very stable, but the functionality of
+its elements is extended every time someone need to operate on the AST. Another
+aspect of the \name{Eclipse} implementation is that it is a public API, and the
+visitor pattern is an easy way to provide access to the nodes in the tree.
The version of the visitor pattern implemented for the AST nodes in \name{Eclipse} also
provides an elegant way to traverse the tree. It does so by following the
before it also makes all its children accept the visitor. The children are only
visited if the visit method of their parent returns \var{true}. This pattern
then makes for a prefix traversal of the AST. If postfix traversal is desired,
-the visitors also has \method{endVisit} methods for each node type, that is
+the visitors also have \method{endVisit} methods for each node type, which is
called after the \method{visit} method for a node. In addition to these visit
methods, there are also the methods \method{preVisit(ASTNode)},
\method{postVisit(ASTNode)} and \method{preVisit2(ASTNode)}. The
\refa{Extract and Move Method} refactoring. It visits expression
statements\typeref{org.eclipse.jdt.core.dom.ExpressionStatement} and creates
prefixes from its expressions in the case of method invocations. The prefixes
-found is registered with a prefix set, together with all its sub-prefixes.
+found are registered with a prefix set, together with all its sub-prefixes.
\subsection{The UnfixesCollector}\label{unfixes}
The \typewithref{no.uio.ifi.refaktor.extractors.collectors}{UnfixesCollector}
finds unfixes within a selection.
\todoin{Give more technical detail?}
-
-
-\subsection{The ContainsReturnStatementCollector}
-\todoin{Remove section?}
-The
-\typewithref{no.uio.ifi.refaktor.analyze.collectors}{ContainsReturnStatementCollector}
-is a very simple property collector. It only visits the return statements within
-a selection, and can report whether it encountered a return statement or not.
-
-\subsection{The LastStatementCollector}
-The \typewithref{no.uio.ifi.refaktor.analyze.collectors}{LastStatementCollector}
-collects the last statement of a selection. It does so by only visiting the top
-level statements of the selection, and compares the textual end offset of each
-encountered statement with the end offset of the previous statement found.
-
\section{Checkers}\label{checkers}
-The checkers are a range of classes that checks that text selections complies
+The checkers are a range of classes that checks that text selections comply
with certain criteria. All checkers operates under the assumption that the code
they check is free from compilation errors. If a
\typewithref{no.uio.ifi.refaktor.analyze.analyzers}{Checker} fails, it throws a
\subsection{The DoubleClassInstanceCreationChecker}
The \type{DoubleClassInstanceCreationChecker} checks that there are no double
-class instance creations where the inner constructor call take and argument that
+class instance creations where the inner constructor call takes an argument that
is built up using field references.
The checker visits all nodes of type \type{ClassInstanceCreation} within a
\subsection{The InstantiationOfNonStaticInnerClassChecker}
The \type{InstantiationOfNonStaticInnerClassChecker} checks that selections
-does not contain instantiations of non-static inner classes. The
+do not contain instantiations of non-static inner classes. The
\type{MoveInstanceMethodProcessor} in \name{Eclipse} does not handle such
instantiations gracefully when moving a method. This problem is also related to
bug\ldots \todoin{File Eclipse bug report}
First, the checker needs to collect some data. Those data are the binding keys
for all simple names that are assigned to within the selection, including
-variable declarations, but excluding fields. The checker also collects whether
-there exists a return statement in the selection or not. No further checks of
-return statements are needed, since, at this point, the selection is already
-checked for illegal return statements \see{returnStatementsChecker}.
+variable declarations, but excluding fields. The checker also finds out whether
+a return statement is found in the selection or not. No further checks of return
+statements are needed, since, at this point, the selection is already checked
+for illegal return statements \see{returnStatementsChecker}.
After the binding keys of the assignees are collected, the checker searches the
part of the enclosing method that is after the selection for references whose
\subsection{The IllegalStatementsChecker}
This checker is designed to check for illegal statements.
-Notice that labels in break and continue statements needs some special
-treatment. Since a label does not have any binding information, we have to
-search upwards in the AST to find the \type{LabeledStatement} that corresponds
-to the label from the break or continue statement, and check that it is
-contained in the selection. If the break or continue statement does not have a
-label attached to it, it is checked that its innermost enclosing loop or switch
-statement (break statements only) also is contained in the selection.
+Notice that labels in break and continue statements need some special treatment.
+Since a label does not have any binding information, we have to search upwards
+in the AST to find the \type{LabeledStatement} that corresponds to the label
+from the break or continue statement, and check that it is contained in the
+selection. If the break or continue statement does not have a label attached to
+it, it is checked that its innermost enclosing loop or switch statement (break
+statements only) also is contained in the selection.
-\todoin{Follow the development in the semantics section\ldots}
+\chapter{Technicalities}
-\chapter{Case Studies}
+\section{Source code organization}
+All the parts of this master's project are under version control with
+\name{Git}\footnote{\url{http://git-scm.com/}}.
-In this chapter I am going to present a few case studies. This is done to give
-an impression of how the search-based \ExtractAndMoveMethod refactoring
-performs when giving it a larger project to take on. I will try to answer where
-it lacks, in terms of completeness, as well as showing its effect on refactored
-source code.
+The software written is organized as some \name{Eclipse} plugins. Writing a plugin is
+the natural way to utilize the API of \name{Eclipse}. This also makes it possible to
+provide a user interface to manually run operations on selections in program
+source code or whole projects/packages.
-The first and primary case, is refactoring source code from the \name{Eclipse
-JDT UI} project. The project is chosen because it is a real project, still in
-development, with a large code base that is written by many different people
-through several years. The code is installed in thousands of \name{Eclipse}
-applications worldwide, and must be seen as a good representative for
-professionally written Java source code. It is also the home for most of the JDT
-refactoring code.
+When writing a plugin in \name{Eclipse}, one has access to resources such as the
+current workspace, the open editor and the current selection.
-For the second case, the \ExtractAndMoveMethod refactoring is fed the
-\code{no.uio.ifi.refaktor} project. This is done as a variation of the
-``dogfooding'' methodology, where you use your own tools to do your job, also
-referred to as ``eating your own dog
-food''\citing{harrisonDogfooding2006}.
+The thesis work is contained in the following Eclipse projects:
-\section{The tools}
-For conducting these experiments, three tools are used. Two of the ``tools''
-both uses Eclipse as their platform. The first is our own tool,
-written to be able to run the \ExtractAndMoveMethod refactoring as a batch
-process, analyzing and refactoring many methods after each other. The second is
-JUnit, that is used for running the projects own unit tests on the target code
-both before and after it is refactored. The last tool that is used is a code
-quality management tool, called \name{SonarQube}. It can be used to perform
-different tasks for assuring code quality, but we are only going to take
-advantage of one of its main features, namely Quality profiles.
+\begin{description}
+ \item[no.uio.ifi.refaktor] \hfill \\ This is the main Eclipse plugin
+ project, and contains all of the business logic for the plugin.
-A quality profile is used to define a set of coding rules that a project is
-supposed to comply with. Failure to following these rules will be recorded as
-so-called ``issues'', marked as having one of several degrees of severities,
-ranging from ``info'' to ``blocker'', where the latter one is the most severe.
-The measurements done for these case studies are therefore not presented as
-fine-grained software metrics results, but rather as the number of issues for
-each defined rule.
+ \item[no.uio.ifi.refaktor.tests] \hfill \\
+ This project contains the tests for the main plugin.
-In addition to the coding rules defined through quality profiles, \name{SonarQube}
-calculates the complexity of source code. The metric that is used is cyclomatic
-complexity, developed by Thomas J. McCabe in
-1976\citing{mccabeComplexity1976}. In this metric, functions have an initial
-complexity of 1, and whenever the control flow of a function splits, the
-complexity increases by
-one\footnote{\url{http://docs.codehaus.org/display/SONAR/Metric+definitions}}.
-\name{SonarQube} discriminates between functions and accessors. Accessors
-are methods that are recognized as setters or getters. Accessors are not counted
-in the complexity analysis.
+ \item[no.uio.ifi.refaktor.examples] \hfill \\
+ Contains example code used in testing. It also contains code for managing
+ this example code, such as creating an Eclipse project from it before a test
+ run.
-\section{The \name{SonarQube} quality profile}
-The quality profile that is used with \name{SonarQube} in these case studies has got
-the name \name{IFI Refaktor Case Study} (version 6). The rules defined in the
-profile are chosen because they are the available rules found in \name{SonarQube} that
-measures complexity and coupling. Now follows a description of the rules in the
-quality profile. The values that are set for these rules are listed in
-\myref{tab:qualityProfile1}.
+ \item[no.uio.ifi.refaktor.benchmark] \hfill \\
+ This project contains code for running search based versions of the
+ composite refactoring over selected Eclipse projects.
+
+ \item[no.uio.ifi.refaktor.releng] \hfill \\
+ Contains the rmap, queries and target definitions needed by Buckminster on
+ the Jenkins continuous integration server.
+
+\end{description}
+
+\subsection{The no.uio.ifi.refaktor project}
+
+\subsubsection{no.uio.ifi.refaktor.analyze}
+This package, and its sub-packages, contains code that is used for analyzing
+Java source code. The most important sub-packages are presented below.
\begin{description}
- \item[Avoid too complex class] is a rule that measures cyclomatic complexity
- for every statement in the body of a class, except for setters and getter.
- The threshold value set is its default value of 200.
+ \item[no.uio.ifi.refaktor.analyze.analyzers] \hfill \\
+ This package contains source code analyzers. These are usually responsible
+ for analyzing text selections or running specialized analyzers for different
+ kinds of entities. Their structures are often hierarchical. This means that
+ you have an analyzer for text selections, that in turn is utilized by an
+ analyzer that analyzes all the selections of a method. Then there are
+ analyzers for analyzing all the methods of a type, all the types of a
+ compilation unit, all the compilation units of a package, and, at last, all
+ of the packages in a project.
- \item[Classes should not be coupled to too many other classes ] is a rule that
- measures how many other classes a class depends upon. It does not count the
- dependencies of nested classes. It is meant to promote the Single
- Responsibility Principle. Although not explicitly stated, the rule's metric
- resembles the \metr{Coupling between object classes} (CBO) metric that is
- described by Chidamber and Kemerer in their article \tit{A Metrics Suite for
- Object Oriented Design}\citing{metricsSuite1994}. The max value for the rule
- is chosen on the background of an empirical study by Raed Shatnawi, that
- concludes that the number 9 is the most useful threshold for the CBO
- metric\citing{shatnawiQuantitative2010}. This study is also performed on
- Eclipse source code, so this threshold value should be particularly well
- suited for the Eclipse JDT UI case in this chapter.
+ \item[no.uio.ifi.refaktor.analyze.checkers] \hfill \\
+ A package containing checkers. The checkers are classes used to validate
+ that a selection can be further analyzed and chosen as a candidate for a
+ refactoring. Invalidating properties can be such as usage of inner classes
+ or the need for multiple return values.
- \item[Control flow statements \ldots{} should not be nested too deeply] is
- a rule that is meant to counter ``Spaghetti code''. It measures the nesting
- level of if, for, while, switch and try statements. The nesting levels start
- at 1. The max value set is its default value of 3.
+ \item[no.uio.ifi.refaktor.analyze.collectors] \hfill \\
+ This package contains the property collectors. Collectors are used to gather
+ properties from a text selection. This is mostly properties regarding
+ referenced names and their occurrences. It is these properties that make up
+ the basis for finding the best candidates for a refactoring.
+\end{description}
- \item[Methods should not be too complex] is a rule that measures cyclomatic
- complexity the same way as the ``Avoid too complex class'' rule. The max
- value used is 10, which ``seems like a reasonable, but not magical, upper
- limit``\citing{mccabeComplexity1976}.
+\subsubsection{no.uio.ifi.refaktor.change}
+This package, and its sub-packages, contains functionality for manipulate source
+code.
- \item[Methods should not have too many lines] is a rule that simply measures
- the number of lines in methods. The threshold value of 20 is used for this
- metric. This is based on my own subjective opinions, as the default value of
- 100 seems a bit too loose.
+\begin{description}
+ \item[no.uio.ifi.refaktor.change.changers] \hfill \\
+ This package contains source code changers. They are used to glue together
+ the analysis of source code and the actual execution of the changes.
- \item[NPath Complexity] is a rule that measures the number of possible
- execution paths through a function. The value used is the default value of
- 200, that seems like a recognized threshold for this metric.
+ \item[no.uio.ifi.refaktor.change.executors] \hfill \\
+ The executors that are responsible for making concrete changes are found in
+ this package. They are mostly used to create and execute one or more Eclipse
+ refactorings.
- \item[Too many methods] is a rule that measures the number of methods in a
- class. The threshold value used is the default value of 10.
+ \item[no.uio.ifi.refaktor.change.processors] \hfill \\
+ Contains a refactoring processor for the \MoveMethod refactoring. The code
+ is stolen and modified to fix a bug. The related bug is described in
+ \myref{eclipse_bug_429416}.
\end{description}
+\subsubsection{no.uio.ifi.refaktor.handlers}
+This package contains handlers for the commands defined in the plugin manifest.
-\begin{table}[htb]
- \caption{The \name{IFI Refaktor Case Study} quality profile (version 6).}
- \label{tab:qualityProfile1}
- \centering
- \begin{tabularx}{\textwidth}{@{}>{\bfseries}L{1.5}R{0.5}@{}}
- \toprule
- \textbf{Rule} & \textbf{Max value} \\
- \midrule
- Avoid too complex class & 200 \\
- Classes should not be coupled to too many other classes (Single
- Responsibility Principle) & 9 \\
- Control flow statements \ldots{} should not be nested too deeply &
- 3 \\
- Methods should not be too complex & 10 \\
- Methods should not have too many lines & 20 \\
- NPath Complexity & 200 \\
- Too many methods & 10 \\
-
- \bottomrule
- \end{tabularx}
-\end{table}
-
-\section{The input}
-A precondition for the source code that is going to be the target for a series
-of \ExtractAndMoveMethod refactorings, is that it is organized as an Eclipse
-project. It is also assumed that the code is free from compilation errors.
+\subsubsection{no.uio.ifi.refaktor.prefix}
+This package contains the \type{Prefix} type that is the data representation of
+the prefixes found by the \type{PrefixesCollector}. It also contains the prefix
+set for storing and working with prefixes.
-\section{The experiment}
-For a given project, the first job that is done, is to refactor its source code.
-The refactoring batch job produces three things: The refactored project,
-statistics gathered during the execution of the series of refactorings, and an
-error log describing any errors happening during this execution. See
-\myref{sec:benchmarking} for more information about how the refactorings are
-performed.
+\subsubsection{no.uio.ifi.refaktor.statistics}
+The package contains statistics functionality. Its heart is the statistics
+aspect that is responsible for gathering statistics during the execution of the
+\ExtractAndMoveMethod refactoring.
-After the refactoring process is done, the before- and after-code is analyzed
-with \name{SonarQube}. The analysis results are then stored in a database and
-displayed through a \name{SonarQube} server with a web interface.\todoin{How
-long are these results going to be publicly available?}
+\begin{description}
+ \item[no.uio.ifi.refaktor.statistics.reports] \hfill \\
+ This package contains a simple framework for generating reports from the
+ statistics data generated by the aspect. Currently, the only available
+ report type is a simple text report.
-The before- and after-code is also tested with their own unit tests. This is
-done to discover any changes in the semantic behavior of the refactored code,
-within the limits of these tests.
+\end{description}
-\section{Case 1: The Eclipse JDT UI project}
-This case is the ultimate test for our \ExtractAndMoveMethod refactoring. The
-target source code is massive. With its over 300,000 lines of code and over
-25,000 methods, it is formidable task to perform automated changes on it. There
-should be plenty of situations where things can go wrong, and, as we shall see
-later, they do.
-I will start by presenting some statistics from the refactoring execution,
-before I pick apart the \name{SonarQube} analysis and conclude by commenting on
-the results from the unit tests. The configuration for the experiment is
-specified in \myref{tab:configurationCase1}.
+\subsubsection{no.uio.ifi.refaktor.textselection}
+This package contains the two custom text selections that are used extensively
+throughout the project. One of them is just a subclass of the other, to support
+the use of the memento pattern to optimize the memory usage during benchmarking.
-\begin{table}[htb]
- \caption{Configuration for Case 1.}
- \label{tab:configurationCase1}
- \centering
- \begin{tabularx}{\textwidth}{@{}>{\bfseries}L{0.67}L{1.33}@{}}
- \toprule
- \spancols{2}{Benchmark data} \\
- \midrule
- Launch configuration & CaseStudy.launch \\
- Project & no.uio.ifi.refaktor.benchmark \\
- Repository & gitolite@git.uio.no:ifi-stolz-refaktor \\
- Commit & 43c16c04520746edd75f8dc2a1935781d3d9de6c \\
- \midrule
- \spancols{2}{Input data} \\
- \midrule
- Project & org.eclipse.jdt.ui \\
- Repository & git://git.eclipse.org/gitroot/jdt/eclipse.jdt.ui.git \\
- Commit & f218388fea6d4ec1da7ce22432726c244888bb6b \\
- Branch & R3\_8\_maintenance \\
- Tests suites & org.eclipse.jdt.ui.tests.AutomatedSuite,
- org.eclipse.jdt.ui.tests.refactoring.all.\-AllAllRefactoringTests \\
-
- \bottomrule
- \end{tabularx}
-\end{table}
-\subsection{Statistics}
-The statistics gathered during the refactoring execution is presented in
-\myref{tab:case1Statistics}.
+\subsubsection{no.uio.ifi.refaktor.debugging}
+The package contains a debug utility class. I addition to this, the package
+\code{no.uio.ifi.refaktor.utils.aspects} contains a couple of aspects used for
+debugging purposes.
-\begin{table}[htb]
- \caption{Statistics after batch refactoring the Eclipse JDT UI project with
- the \ExtractAndMoveMethod refactoring.}
- \label{tab:case1Statistics}
- \centering
- \begin{tabularx}{\textwidth}{@{}>{\bfseries}L{1.5}R{0.5}@{}}
- \toprule
- \spancols{2}{Time used} \\
- \midrule
- Total time & 98m38s \\
- Analysis time & 14m41s (15\%) \\
- Change time & 74m20s (75\%) \\
- Miscellaneous tasks & 9m37s (10\%) \\
- \midrule
- \spancols{2}{Numbers of each type of entity analyzed} \\
- \midrule
- Packages & 110 \\
- Compilation units & 2,097 \\
- Types & 3,152 \\
- Methods & 27,667 \\
- Text selections & 591,500 \\
- \midrule
- \spancols{2}{Numbers for \ExtractAndMoveMethod refactoring candidates} \\
- \midrule
- Methods chosen as candidates & 2,552 \\
- Methods NOT chosen as candidates & 25,115 \\
- Candidate selections (multiple per method) & 36,843 \\
- \midrule
- \spancols{2}{\ExtractAndMoveMethod refactorings executed} \\
- \midrule
- Fully executed & 2,469 \\
- Not fully executed & 83 \\
- Total attempts & 2,552 \\
- \midrule
- \spancols{2}{Primitive refactorings executed} \\
- \spancols{2}{\small \ExtractMethod refactorings} \\
- \midrule
- Performed & 2,483 \\
- Not performed & 69 \\
- Total attempts & 2,552 \\
- \midrule
- \spancols{2}{\small \MoveMethod refactorings} \\
- \midrule
- Performed & 2469 \\
- Not performed & 14 \\
- Total attempts & 2,483 \\
+\subsubsection{no.uio.ifi.refaktor.utils}
+Utility package that contains all the functionality that has to do with parsing
+of source code. It also has utility classes for looking up handles to methods
+and types et cetera.
- \bottomrule
- \end{tabularx}
-\end{table}
+\begin{description}
+ \item[no.uio.ifi.refaktor.utils.caching] \hfill \\
+ This package contains the caching manager for compilation units, along with
+ classes for different caching strategies.
-\subsubsection{Execution time}
-I consider the total execution time of approximately 1.5 hours as being
-acceptable. It clearly makes the batch process unsuitable for doing any
-on-demand analysis or changes, but it is good enough for running periodic jobs,
-like over-night analysis.
+ \item[no.uio.ifi.refaktor.utils.nullobjects] \hfill \\
+ Contains classes for creating different null objects. Most of the classes
+ are used to represent null objects of different handle types. These null
+ objects are returned from various utility classes instead of returning a
+ \var{null} value when other values are not available.
-As the statistics show, 75\% of the total time goes into making the actual code
-changes. The time consumers are here the primitive \ExtractMethod and
-\MoveMethod refactorings. Included in the change time is the parsing and
-precondition checking done by the refactorings, as well as textual changes done
-to files on disk. All this parsing and disk access is time-consuming, and
-constitute a large part of the change time.
+\end{description}
-In comparison, the pure analysis time, used to find suitable candidates, only
-make up for 15\% of the total time consumed. This includes analyzing almost
-600,000 text selections, while the number of attempted executions of the
-\ExtractAndMoveMethod refactoring are only about 2,500. So the number of
-executed primitive refactorings are approximately 5,000. Assuming the time used
-on miscellaneous tasks are used mostly for parsing source code for the analysis,
-we can say that the time used for analyzing code is at most 25\% of the total
-time. This means that for every primitive refactoring executed, we can analyze
-around 360 text selections. So, with an average of about 21 text selections per
-method, it is reasonable to say that we can analyze over 15 methods in the time
-it takes to perform a primitive refactoring.
+\section{Continuous integration}
+The continuous integration server
+\name{Jenkins}\footnote{\url{http://jenkins-ci.org/}} has been set up for the
+project\footnote{A work mostly done by the supervisor.}. It is used as a way to
+run tests and perform code coverage analysis.
-\subsubsection{Refactoring candidates}
-Out of the 27,667 methods that were analyzed, 2,552 methods contained selections
-that were considered candidates for the \ExtractAndMoveMethod refactoring. This
-is roughly 9\% off the methods in the project. These 9\% of the methods had on
-average 14.4 text selections that were considered possible refactoring
-candidates.
+To be able to build the \name{Eclipse} plugins and run tests for them with Jenkins, the
+component assembly project
+\name{Buckminster}\footnote{\url{http://www.eclipse.org/buckminster/}} is used,
+through its plugin for Jenkins. Buckminster provides for a way to specify the
+resources needed for building a project and where and how to find them.
+Buckminster also handles the setup of a target environment to run the tests in.
+All this is needed because the code to build depends on an \name{Eclipse}
+installation with various plugins.
-\subsubsection{Executed refactorings}
-2,469 out of 2,552 attempts on executing the \ExtractAndMoveMethod refactoring
-were successful, giving a success rate of 96.7\%. The failure rate of 3.3\% stem
-from situations where the analysis finds a candidate selection, but the change
-execution fails. This failure could be an exception that was thrown, and the
-refactoring aborts. It could also be the precondition checking for one of the
-primitive refactorings that gives us an error status, meaning that if the
-refactoring proceeds, the code will contain compilation errors afterwards,
-forcing the composite refactoring to abort. This means that if the
-\ExtractMethod refactoring fails, no attempt is done for the \MoveMethod
-refactoring. \todo{Redundant information? Put in benchmark chapter?}
+\subsection{Problems with AspectJ}
+The Buckminster build worked fine until introducing AspectJ into the project.
+When building projects using AspectJ, there are some additional steps that need
+to be performed. First of all, the aspects themselves must be compiled. Then the
+aspects need to be woven with the classes they affect. This demands a process
+that does multiple passes over the source code.
-Out of the 2,552 \ExtractMethod refactorings that were attempted executed, 69 of
-them failed. This give a failure rate of 2.7\% for the primitive refactoring. In
-comparison, the \MoveMethod refactoring had a failure rate of 0.6 \% of the
-2,483 attempts on the refactoring.
+When using AspectJ with \name{Eclipse}, the specialized compilation and the
+weaving can be handled by the \name{AspectJ Development
+Tools}\footnote{\url{https://www.eclipse.org/ajdt/}}. This works all fine, but
+it complicates things when trying to build a project depending on \name{Eclipse}
+plugins outside of \name{Eclipse}. There is supposed to be a way to specify a
+compiler adapter for javac, together with the file extensions for the file types
+it shall operate. The AspectJ compiler adapter is called
+\typewithref{org.aspectj.tools.ant.taskdefs}{Ajc11CompilerAdapter}, and it works
+with files that has the extensions \code{*.java} and \code{*.aj}. I tried to
+setup this in the build properties file for the project containing the aspects,
+but to no avail. The project containing the aspects does not seem to be built at
+all, and the projects that depend on it complain that they cannot find certain
+classes.
+
+I then managed to write an \name{Ant}\footnote{\url{https://ant.apache.org/}}
+build file that utilizes the AspectJ compiler adapter, for the
+\code{no.uio.ifi.refaktor} plugin. The problem was then that it could no longer
+take advantage of the environment set up by Buckminster. The solution to this
+particular problem was of a ``hacky'' nature. It involves exporting the plugin
+dependencies for the project to an Ant build file, and copy the exported path
+into the existing build script. But then the Ant script needs to know where the
+local \name{Eclipse} installation is located. This is no problem when building
+on a local machine, but to utilize the setup done by Buckminster is a problem
+still unsolved. To get the classpath for the build setup correctly, and here
+comes the most ``hacky'' part of the solution, the Ant script has a target for
+copying the classpath elements into a directory relative to the project
+directory and checking it into Git. When no \code{ECLIPSE\_HOME} property is set
+while running Ant, the script uses the copied plugins instead of the ones
+provided by the \name{Eclipse} installation when building the project. This
+obviously creates some problems with maintaining the list of dependencies in the
+Ant file, as well as remembering to copy the plugins every time the list of
+dependencies changes.
+
+The Ant script described above is run by Jenkins before the Buckminster setup
+and build. When setup like this, the Buckminster build succeeds for the projects
+not using AspectJ, and the tests are run as normal. This is all good, but it
+feels a little scary, since the reason for Buckminster not working with AspectJ
+is still unknown.
+
+The problems with building with AspectJ on the Jenkins server lasted for a
+while, before they were solved. This is reflected in the ``Test Result Trend''
+and ``Code Coverage Trend'' reported by Jenkins.
+
+\chapter{Benchmarking}\label{sec:benchmarking}
+This part of the master's project is located in the \name{Eclipse} project
+\code{no.uio.ifi.refaktor.benchmark}. The purpose of it is to run the equivalent
+of the \type{SearchBasedExtractAndMoveMethodChanger}
+\see{searchBasedExtractAndMoveMethodChanger} over a larger software project,
+both to test its robustness but also its effect on different software metrics.
+
+\section{The benchmark setup}
+The benchmark itself is set up as a \name{JUnit} test case. This is a convenient
+setup, and utilizes the \name{JUnit Plugin Test Launcher}. This provides us with
+a fully functional \name{Eclipse} workbench. Most importantly, this gives us
+access to the Java Model of \name{Eclipse} \see{javaModel}.
+
+\subsection{The ProjectImporter}
+The Java project that is going to be used as the data for the benchmark, must be
+imported into the JUnit workspace. This is done by the
+\typewithref{no.uio.ifi.refaktor.benchmark}{ProjectImporter}. The importer
+requires the absolute path to the project description file. This file is named
+\code{.project} and is located at the root of the project directory.
+
+The project description is loaded to find the name of the project to be
+imported. The project that shall be the destination for the import is created in
+the workspace, on the base of the name from the description. Then an import
+operation is created, based on both the source and destination information. The
+import operation is run to perform the import.
+
+I have found no simple API call to accomplish what the importer does, which
+tells me that it may not be too many people performing this particular action.
+The solution to the problem was found on \name{Stack
+Overflow}\footnote{\url{https://stackoverflow.com/questions/12401297}}. It
+contains enough dirty details to be considered inconvenient to use, if not
+wrapping it in a class like my \type{ProjectImporter}. One would probably have
+to delve into the source code for the import wizard to find out how the import
+operation works, if no one had already done it.
+
+\section{Statistics}
+Statistics for the analysis and changes is captured by the
+\typewithref{no.uio.ifi.refaktor.aspects}{StatisticsAspect}. This an
+\emph{aspect} written in \name{AspectJ}.
+
+\subsection{AspectJ}
+\name{AspectJ}\footnote{\url{http://eclipse.org/aspectj/}} is an extension to
+the Java language, and facilitates combining aspect-oriented programming with
+the object-oriented programming in Java.
+
+Aspect-oriented programming is a programming paradigm that is meant to isolate
+so-called \emph{cross-cutting concerns} into their own modules. These
+cross-cutting concerns are functionalities that span over multiple classes, but
+may not belong naturally in any of them. It can be functionality that does not
+concern the business logic of an application, and thus may be a burden when
+entangled with parts of the source code it does not really belong. Examples
+include logging, debugging, optimization and security.
+
+Aspects are interacting with other modules by defining advices. The concept of
+an \emph{advice} is known from both aspect-oriented and functional
+programming\citing{wikiAdvice2014}. It is a function that modifies another
+function when the latter is run. An advice in AspectJ is somewhat similar to a
+method in Java. It is meant to alter the behavior of other methods, and contains
+a body that is executed when it is applied.
+
+An advice can be applied at a defined \emph{pointcut}. A pointcut picks out one
+or more \emph{join points}. A join point is a well-defined point in the
+execution of a program. It can occur when calling a method defined for a
+particular class, when calling all methods with the same name,
+accessing/assigning to a particular field of a given class and so on. An advice
+can be declared to run both before, after returning from a pointcut, when there
+is thrown an exception in the pointcut or after the pointcut either returns or
+throws an exception. In addition to picking out join points, a pointcut can
+also bind variables from its context, so they can be accessed in the body of an
+advice. An example of a pointcut and an advice is found in
+\myref{lst:aspectjExample}.
+
+\begin{listing}[h]
+\begin{minted}{aspectj}
+pointcut methodAnalyze(
+ SearchBasedExtractAndMoveMethodAnalyzer analyzer) :
+ call(* SearchBasedExtractAndMoveMethodAnalyzer.analyze())
+ && target(analyzer);
+
+after(SearchBasedExtractAndMoveMethodAnalyzer analyzer) :
+ methodAnalyze(analyzer) {
+ statistics.methodCount++;
+ debugPrintMethodAnalysisProgress(analyzer.method);
+}
+\end{minted}
+\caption{An example of a pointcut named \method{methodAnalyze},
+and an advice defined to be applied after it has occurred.}
+\label{lst:aspectjExample}
+\end{listing}
+
+\subsection{The Statistics class}
+The statistics aspect stores statistical information in an object of type
+\type{Statistics}. As of now, the aspect needs to be initialized at the point in
+time where it is desired that it starts its data gathering. At any point in time
+the statistics aspect can be queried for a snapshot of the current statistics.
+
+The \type{Statistics} class also includes functionality for generating a report
+of its gathered statistics. The report can be given either as a string or it can
+be written to a file.
+
+\subsection{Advices}
+The statistics aspect contains advices for gathering statistical data from
+different parts of the benchmarking process. It captures statistics from both
+the analysis part and the execution part of the composite \ExtractAndMoveMethod
+refactoring.
+
+For the analysis part, there are advices to count the number of text selections
+analyzed and the number of methods, types, compilation units and packages
+analyzed. There are also advices that counts for how many of the methods there
+are found a selection that is a candidate for the refactoring, and for how many
+methods there are not.
+
+There exist advices for counting both the successful and unsuccessful executions
+of all the refactorings. Both for the \ExtractMethod and \MoveMethod
+refactorings in isolation, as well as for the combination of them.
+
+\section{Optimizations}
+When looking for possible optimizations for the benchmarking process, I used the
+\name{VisualVM}\footnote{\url{http://visualvm.java.net/}} \gloss{profiler} for
+the Java Virtual Machine to both profile the application and also to make memory
+dumps of its heap.
+
+\subsection{Caching}
+When \gloss{profiling} the benchmark process before making any optimizations, it
+early became apparent that the parsing of source code was a place to direct
+attention towards. This discovery was done when only \emph{analyzing} source
+code, before trying to do any \emph{manipulation} of it. Caching of the parsed
+ASTs seemed like the best way to save some time, as expected. With only a simple
+cache of the most recently used AST, the analysis time was speeded up by a
+factor of around 20. This number depends a little upon which type of system the
+analysis is run.
+
+The caching is managed by a cache manager, that now, by default, utilizes the
+not so well known feature of Java called a \emph{soft reference}. Soft
+references are best explained in the context of weak references. A \emph{weak
+reference} is a reference to an object instance that is only guaranteed to
+persist as long as there is a \emph{strong reference} or a soft reference
+referring the same object. If no such reference is found, its referred object is
+garbage collected. A strong reference is basically the same as a regular Java
+reference. A soft reference has the same guarantees as a week reference when it
+comes to its relation to strong references, but it is not necessarily garbage
+collected if there are no strong references to it. A soft reference \emph{may}
+reside in memory as long as the JVM has enough free memory in the heap. A soft
+reference will therefore usually perform better than a weak reference when used
+for simple caching and similar tasks. The way to use a soft/weak reference is to
+as it for its referent. The return value then has to be tested to check that it
+is not \var{null}. For the basic usage of soft references, see
+\myref{lst:softReferenceExample}. For a more thorough explanation of weak
+references in general, see\citing{weakRef2006}.
+
+\begin{listing}[h]
+\begin{minted}{java}
+// Strong reference
+Object strongRef = new Object();
+
+// Soft reference
+SoftReference<Object> softRef =
+ new SoftReference<Object>(new Object());
+
+// Using the soft reference
+Object obj = softRef.get();
+if (obj != null) {
+ // Use object here
+}
+\end{minted}
+\caption{Showing the basic usage of soft references. Weak references is used the
+ same way. {\footnotesize (The references are part of the \code{java.lang.ref}
+package.)}}
+\label{lst:softReferenceExample}
+\end{listing}
+
+The cache based on soft references has no limit for how many ASTs it caches. It
+is generally not advisable to keep references to ASTs for prolonged periods of
+time, since they are expensive structures to hold on to. For regular plugin
+development, \name{Eclipse} recommends not creating more than one AST at a time to
+limit memory consumption. Since the benchmarking has nothing to do with user
+experience, and throughput is everything, these advices are intentionally
+ignored. This means that during the benchmarking process, the target \name{Eclipse}
+application may very well work close to its memory limit for the heap space for
+long periods during the benchmark.
+
+\subsection{Candidates stored as mementos}
+When performing large scale analysis of source code for finding candidates to
+the \ExtractAndMoveMethod refactoring, memory is an issue. One of the inputs to
+the refactoring is a variable binding. This variable binding indirectly retains
+a whole AST. Since ASTs are large structures, this quickly leads to an
+\type{OutOfMemoryError} if trying to analyze a large project without optimizing
+how we store the candidates' data. This means that the JVM cannot allocate more
+memory for our benchmark, and it exits disgracefully.
+
+A possible solution could be to just allow the JVM to allocate even more memory,
+but this is not a dependable solution. The allocated memory could easily
+supersede the physical memory of a machine, which would make the benchmark go
+really slow.
+
+Thus, the candidates' data must be stored in another format. Therefore, we use
+the \gloss{mementoPattern} to store variable binding information. This is done
+in a way that makes it possible to retrieve a variable binding at a later point.
+The data that is stored to achieve this, is the key to the original variable
+binding. In addition to the key, we know which method and text selection the
+variable is referenced in, so that we can find it by parsing the source code and
+search for it when it is needed.
+
+\section{Handling failures}
+\todoin{write}
+
+
+\chapter{Case studies}
+
+In this chapter I am going to present a few case studies. This is done to give
+an impression of how the search-based \ExtractAndMoveMethod refactoring
+performs when giving it a larger project to take on. I will try to answer where
+it lacks, in terms of completeness, as well as showing its effect on refactored
+source code.
+
+The first and primary case, is refactoring source code from the \name{Eclipse
+JDT UI} project. The project is chosen because it is a well-known open-source
+project, still in development, with a large code base that is written by many
+different people over several years. The code is installed in a large number of
+\name{Eclipse} applications worldwide, and many other projects build on the
+Eclipse platform. For a long time, it was even the official IDE for Android
+development. All this means that Eclipse must be seen as a good representative
+for professionally written Java source code. It is also the home for most of the
+JDT refactoring code.
+
+For the second case, the \ExtractAndMoveMethod refactoring is fed the
+\code{no.uio.ifi.refaktor} project. This is done as a variation of the
+``dogfooding'' methodology.
+
+\section{The tools}
+For conducting these experiments, three software tools are used. Two of the
+tools both use Eclipse as their platform. The first is our own tool, described
+in \myref{sec:benchmarking}, written to be able to run the \ExtractAndMoveMethod
+refactoring as a batch process. It analyzes and refactors all the methods of a
+project in sequence. The second is JUnit, which is used for running the
+project's own unit tests on the target code both before and after it is
+refactored. The last tool that is used is a code quality management tool, called
+\name{SonarQube}. It can be used to perform different tasks for assuring code
+quality, but we are only going to take advantage of one of its main features,
+namely quality profiles.
+
+A quality profile is used to define a set of coding rules that a project is
+supposed to comply with. Failure to following these rules will be recorded as
+so-called ``issues'', marked as having one of several degrees of severities,
+ranging from ``info'' to ``blocker'', where the latter one is the most severe.
+The measurements done for these case studies are therefore not presented as
+fine-grained software metrics results, but rather as the number of issues for
+each defined rule.
+
+In its analysis, \name{SonarQube} discriminates between functions and accessors.
+Accessors are methods that are recognized as setters or getters.
+
+In addition to the coding rules defined through quality profiles,
+\name{SonarQube} calculates the complexity of source code. The metric that is
+used is cyclomatic complexity, developed by Thomas J. McCabe in
+1976\citing{mccabeComplexity1976}. In this metric, functions have an initial
+complexity of 1, and whenever the control flow of a function splits, the
+complexity increases by
+one\footnote{\url{http://docs.codehaus.org/display/SONAR/Metric+definitions}}.
+Accessors are not counted in the complexity analysis.
-\subsection{\name{SonarQube} analysis}
-Results from the \name{SonarQube} analysis is shown in
-\myref{tab:case1ResultsProfile1}.
+Specifications for the computer used during the experiments are shown in
+\myref{tab:experimentComputerSpecs}.
\begin{table}[htb]
- \caption{Results for analyzing the Eclipse JDT UI project, before and after
- the refactoring, with \name{SonarQube} and the \name{IFI Refaktor Case Study}
- quality profile. (Bold numbers are better.)}
- \label{tab:case1ResultsProfile1}
+ \caption{Specifications for experiment computer.}
+ \label{tab:experimentComputerSpecs}
\centering
- \begin{tabularx}{\textwidth}{@{}>{\bfseries}L{1.5}R{0.25}R{0.25}@{}}
+ \begin{tabularx}{\textwidth}{@{}>{\bfseries}L{0.35}R{1.65}@{}}
\toprule
- \textnormal{Number of issues for each rule} & Before & After \\
- \midrule
- Avoid too complex class & 81 & \textbf{79} \\
- Classes should not be coupled to too many other classes (Single
- Responsibility Principle) & \textbf{1,098} & 1,199 \\
- Control flow statements \ldots{} should not be nested too deeply & 1,375 &
- \textbf{1,285} \\
- Methods should not be too complex & 1,518 & \textbf{1,452} \\
- Methods should not have too many lines & 3,396 & \textbf{3,291} \\
- NPath Complexity & 348 & \textbf{329} \\
- Too many methods & \textbf{454} & 520 \\
- \midrule
- Total number of issues & 8,270 & \textbf{8,155} \\
- \midrule
- \midrule
- \spancols{3}{Complexity} \\
- \midrule
- Per function & 3.6 & \textbf{3.3} \\
- Per class & \textbf{29.5} & 30.4 \\
- Per file & \textbf{44.0} & 45.3 \\
- \midrule
- Total complexity & \textbf{84,765} & 87,257 \\
- \midrule
+ \spancols{2}{Hardware} \\
\midrule
- \spancols{3}{Numbers of each type of entity analyzed} \\
+ Model & Lenovo ThinkPad Edge S430 \\
+ Processor & Intel\textregistered{} Core\texttrademark{}
+ i5-3210M\linebreak[4] (2.5 GHz/3.1 GHz (turbo),
+ 2 cores, 4 threads, 3 MB Cache) \\
+ Memory & 8 GB DDR3 1600 MHz \\
+ Storage & 500 GB HDD (7200 RPM) + 16 GB SSD Cache for Lenovo Hard Disk Drive
+ Performance Booster \\
\midrule
- Files & 1,926 & 1,926 \\
- Classes & 2,875 & 2,875 \\
- Functions & 23,744 & 26,332 \\
- Accessors & 1,296 & 1,019 \\
- Statements & 162,768 & 165,145 \\
- Lines of code & 320,941 & 329,112 \\
+ \spancols{2}{Operating system} \\
\midrule
- Technical debt (in days) & \textbf{1,003.4} & 1,032.7 \\
+ Distribution & Ubuntu 12.10 \\
+ Kernel & Linux 3.5.0-49-generic (x86\_64) \\
+
\bottomrule
\end{tabularx}
\end{table}
-\subsubsection{Diversity in the number of entities analyzed}
-The analysis performed by \name{SonarQube} is reporting fewer methods than found
-by the pre-refactoring analysis. \name{SonarQube} discriminates between
-functions (methods) and accessors, so the 1,296 accessors play a part in this
-calculation. \name{SonarQube} also has the same definition as our plugin when
-it comes to how a class is defined. Therefore is seems like \name{SonarQube}
-misses 277 classes that our plugin handles. This can explain why the {SonarQube}
-report differs from our numbers by approximately 2,500 methods,
-
-\subsubsection{Complexity}
-On all complexity rules that works on the method level, the number of issues
-decreases with between 3.1\% and 6.5\% from before to after the refactoring. The
-average complexity of a method decreases from 3.6 to 3.3, which is an
-improvement of about 8.3\%. So, on the method level, the refactoring must be
-said to have a slightly positive impact.
-
-The improvement in complexity on the method level is somewhat traded for
-complexity on the class level. The complexity per class metric is worsen by 3\%
-from before to after. The issues for the ``Too many methods'' rule also
-increases by 14.5\%. These numbers indicate that the refactoring makes quite a
-lot of the classes a little more complex overall. This is the expected outcome,
-since the \ExtractAndMoveMethod refactoring introduces almost 2,500 new methods
-into the project.
-
-The only number that can save the refactoring's impact on complexity on the
-class level, is the ``Avoid too complex class'' rule. It improves with 2.5\%,
-thus indicating that the complexity is moderately better distributed between the
-classes after the refactoring than before.
-
-\subsubsection{Coupling}
-One of the hopes when starting this project, was to be able to make a
-refactoring that could lower the coupling between classes. Better complexity at
-the method level is a not very unexpected byproduct of dividing methods into
-smaller parts. Lowering the coupling on the other hand, is a far greater task.
-This is also reflected in the results for the only coupling rule defined in the
-\name{SonarQube} quality profile, namely the ``Classes should not be coupled to
-too many
-other classes (Single Responsibility Principle)'' rule.
-
-The number of issues for the coupling rule is 1,098 before the refactoring, and
-1,199 afterwards. This is an increase in issues of 9.2\%, and a blow for this
-project. These numbers can be interpreted two ways. The first possibility is
-that our assumptions are wrong, and that increasing indirection does not
-decrease coupling between classes. The other possibility is that our analysis
-and choices of candidate text selections are not good enough. I vote for the
-second possibility. (Voting against the public opinion may also be a little
-bold.)
-
-What probably happens is, that many of the times the \ExtractAndMoveMethod
-refactoring is performed, the \MoveMethod refactoring ``drags'' with it
-references to classes that are unknown to the method destination. If it happens
-to be so lucky that it removes a dependency from one class, it might as well
-introduce three new dependencies to another class. In those situations that a
-class does not know about the originating class of a moved method, the
-\MoveMethod refactoring most certainly will introduce a dependency. This is
-because there is a
-bug\footnote{\url{https://bugs.eclipse.org/bugs/show\_bug.cgi?id=228635}} in the
-refactoring, making it pass an instance of the originating class as a reference
-to the moved method, regardless of whether the reference is used in the method
-body or not.
-There is also the possibility that the heuristics used to find candidate text
-selections are not good enough, they most certainly are not. I wish I had more
-time to fine-tune them, and to complete the analysis part of the project, but
-this is simply not the case. This becomes even clearer when analyzing the unit
-test results for the after-code.
+\section{The \name{SonarQube} quality profile}
+The quality profile that is used with \name{SonarQube} in these case studies has got
+the name \name{IFI Refaktor Case Study} (version 6). The rules defined in the
+profile are chosen because they are the available rules found in \name{SonarQube} that
+measures complexity and coupling. Now follows a description of the rules in the
+quality profile. The values that are set for these rules are listed in
+\myref{tab:qualityProfile1}.
-\subsubsection{Totals}
-On the bright side, the total number of issues is lower after the refactoring
-than it was before. Before the refactoring, the total number of issues is
-8,270, and after it is 8,155. An improvement of only 1.4\%.
+\begin{description}
+ \item[Avoid too complex class] is a rule that measures cyclomatic complexity
+ for every statement in the body of a class, except for setters and getter.
+ The threshold value set is its default value of 200.
-Then \name{SonarQube} tells me that the total complexity has increased by
-2.9\%, and that the (more questionable) ``technical debt'' has increased from
-1,003.4 to 1,032.7 days, also a deterioration of 2.9\%. Although these numbers
-are similar, no correlation has been found between them.
+ \item[Classes should not be coupled to too many other classes ] is a rule that
+ measures how many other classes a class depends upon. It does not count the
+ dependencies of nested classes. It is meant to promote the Single
+ Responsibility Principle. The metric for the rule resembles the CBO metric
+ that is described in \myref{sec:CBO}, but is only considering outgoing
+ dependencies. The max value for the rule is chosen on the basis of an
+ empirical study by Raed Shatnawi, which concludes that the number 9 is the
+ most useful threshold for the CBO metric\citing{shatnawiQuantitative2010}.
+ This study is also performed on Eclipse source code, so this threshold value
+ should be particularly well suited for the Eclipse JDT UI case in this
+ chapter.
-\subsection{Unit tests}
-The tests that have been run for the \name{Eclipse JDT UI} project, are the
-tests in the test suites specified as the main test suites on the JDT UI wiki
-page on how to contribute to the
-project\footnote{\url{https://wiki.eclipse.org/JDT\_UI/How\_to\_Contribute\#Unit\_Testing}}.
+ \item[Control flow statements \ldots{} should not be nested too deeply] is
+ a rule that is meant to counter ``Spaghetti code''. It measures the nesting
+ level of \emph{if}, \emph{for}, \emph{while}, \emph{switch} and \emph{try}
+ statements. The nesting levels start at 1. The max value set is its default
+ value of 3.
-\subsubsection{Before the refactoring}
-Running the tests for the before-code of Eclipse JDT UI yielded 4 errors and 3
-failures for the \type{AutomatedSuite} test suite (2,007 test cases), and 2
-errors and
-3 failures for the \type{AllAllRefactoringTests} test suite (3,816 test cases).
+ \item[Methods should not be too complex] is a rule that measures cyclomatic
+ complexity the same way as the ``Avoid too complex class'' rule. The max
+ value used is 10, which ``seems like a reasonable, but not magical, upper
+ limit``\citing{mccabeComplexity1976}.
-\subsubsection{After the refactoring}
-The test results for the after-code of the Eclipse JDT UI project is another
-story. The reason for this is that during the setup for the unit tests, Eclipse
-now reports that the project contains 322 fatal errors, and a lot of errors that
-probably follows from these. This is another blow for this master's project.
-
-It has now been shown that the \ExtractAndMoveMethod refactoring, in its current
-state, produces code that does not compile. Had these errors originated from
-only one bug, it would not have been much of a problem, but this is not the
-case. By only looking at some random compilation problems in the refactored code,
-I came up with at least four different bugs \todo{write bug reports?} that
-caused those problems. I then stopped looking for more, since some of the bugs
-would take more time to fix than I could justify using on them at this point.
-
-The only thing that can be said in my defence, is that all the compilation
-errors could have been avoided if the types of situations that causes them were
-properly handled by the primitive refactorings, that again are supported by the
-Eclipse JDT UI project. All of the four randomly found bugs that I mentioned
-before, are also weaknesses of the \MoveMethod refactoring. If the primitive
-refactorings had detected the up-coming errors
-in their precondition checking phase, the refactorings would have been aborted,
-since this is how the \ExtractAndMoveMethod refactoring handles such situations.
-
-Of course, taking all possible situations into account is an immense task. This
-is one of the reasons for the failure. A complete analysis is too big of a task
-for this master's project to handle. Looking at it now, this comes as no
-surprise, since the task is obviously also too big for the creators of the
-primitive \MoveMethod refactoring. This shows that the underlying primitive
-refactorings are not complete enough to be fully relied upon for avoiding
-compilation errors.
+ \item[Methods should not have too many lines] is a rule that simply measures
+ the number of lines in methods. A threshold value of 20 is used for this
+ metric. This is based on my own subjective opinions, as the default value of
+ 100 describes method bodies that do not even fit on most screens.
-Considering all these problems, it is difficult to know how to interpret the
-unit test results from after refactoring the Eclipse JDT UI. The
-\type{AutomatedSuite} reported 565 errors and 5 failures. Three of the failures
-were the same as reported before the refactoring took place, so two of them are
-new. For these two cases it is not immediately apparent what makes them behave
-differently. The program is so complex that to analyze it to find this out, we
-might need more powerful methods than just manually analyzing its source code.
-This is somewhat characteristic for imperative programming: The programs are
-often hard to analyze and understand.
+ \item[NPath Complexity] is a rule that measures the number of possible
+ execution paths through a function. The value used is the default value of
+ 200, which seems like a recognized threshold for this metric.
-For the \type{AllAllRefactoringTests} test suite, the three failures are gone,
-but the two errors have grown to 2,257 errors. I will not try to analyze those
-errors.
+ \item[Too many methods] is a rule that measures the number of methods in a
+ class. The threshold value used is the default value of 10.
-What I can say, is that it is likely that the \ExtractAndMoveMethod refactoring
-has introduced some unintended behavioral changes. Let us say that the
-refactoring introduces at least two behavior-altering changes for every 2,500
-executions. More than that is difficult to say about the behavior-preserving
-properties of the \ExtractAndMoveMethod refactoring, at this point.
+\end{description}
-\subsection{Conclusions}
-After automatically analyzing and executing the \ExtractAndMoveMethod
-refactoring for all the methods in the Eclipse JDT UI project, the results does
-not look that promising. For this case, the refactoring seems almost unusable as
-it is now. The error rate and measurements done tells us this.
-The refactoring makes the code a little less complex at the method level. But
-this is merely a side effect of extracting methods, and holds little scientific
-value. When it comes to the overall complexity, it is increased, although it is
-slightly better spread among the classes.
+\begin{table}[htb]
+ \caption{The \name{IFI Refaktor Case Study} quality profile (version 6).}
+ \label{tab:qualityProfile1}
+ \centering
+ \begin{tabularx}{\textwidth}{@{}>{\bfseries}L{1.5}R{0.5}@{}}
+ \toprule
+ \textbf{Rule} & \textbf{Max value} \\
+ \midrule
+ Avoid too complex class & 200 \\
+ Classes should not be coupled to too many other classes (Single
+ Responsibility Principle) & 9 \\
+ Control flow statements \ldots{} should not be nested too deeply &
+ 3 \\
+ Methods should not be too complex & 10 \\
+ Methods should not have too many lines & 20 \\
+ NPath Complexity & 200 \\
+ Too many methods & 10 \\
+
+ \bottomrule
+ \end{tabularx}
+\end{table}
-The analysis done before the \ExtractAndMoveMethod refactoring, is currently not
-complete enough to make the refactoring useful. It introduces too many errors in
-the code, and the code may change it's behavior. It also remains to prove that
-large scale refactoring with it can decrease coupling between classes. A better
-analysis may prove this, but in its present state, the opposite is the fact. The
-coupling measurements done by \name{SonarQube} shows this.
+\section{The input}
+A precondition for the source code that is going to be the target for a series
+of \ExtractAndMoveMethod refactorings, is that it is organized as an Eclipse
+project. It is also assumed that the code is free from compilation errors.
-On the bright side, the performance of the refactoring process is not that bad.
-It shows that it is possible to make a tool the way we do, if we can make the
-tool do anything useful. As long as the analysis phase is not going to involve
-anything that uses to much disk access, a lot of analysis can be done in a
-reasonable amount of time.
+\section{The experiment}
+For a given project, the first job that is done, is to refactor its source code.
+The refactoring batch job produces three things: The refactored project,
+statistics gathered during the execution of the series of refactorings, and an
+error log describing any errors happening during this execution. See
+\myref{sec:benchmarking} for more information about how the refactorings are
+performed.
-The time used on performing the actual changes excludes a trial and error
-approach with the tools used in this master's project. In a trial and error
-approach, you could for instance be using the primitive refactorings used in
-this project to refactor code, and only then make decisions based on the effect,
-possibly shown by traditional software metrics. The problem with the approach
-taken in this project, compared to a trial and error approach, is that using
-heuristics beforehand is much more complicated. But on the other hand, a trial
-and error approach would still need to face the challenges of producing code
-that does compile without errors. If using refactorings that could produce
-in-memory changes, a trial and error approach could be made more efficient.
+After the refactoring process is done, the before- and after-code is analyzed
+with \name{SonarQube}. The analysis results are then stored in a database and
+displayed through a \name{SonarQube} server with a web interface.
-\section{Case 2: The \type{no.uio.ifi.refaktor} project}
-In this case we will see a form of the ``dogfooding'' methodology used, when
-refactoring our own \type{no.uio.ifi.refaktor} project with the
-\ExtractAndMoveMethod refactoring.
+The before- and after-code is also tested with their own unit tests. This is
+done to discover any changes in the semantic behavior of the refactored code,
+within the limits of these tests.
-In this case I will try to point out some differences from case 1, and how they
-impact the execution of the benchmark. The refaktor project is 39 times smaller
-than the Eclipse JDT UI project, measured in lines of code. This will make
-things a bit more transparent. It will therefore be interesting to see if this
-case can shed light on any aspect of our project that were lost in the larger
-case 1.
+\section{Case 1: The Eclipse JDT UI project}
+This case is the ultimate test for our \ExtractAndMoveMethod refactoring. The
+target source code is massive. With its over 300,000 lines of code\footnote{For
+ all uses of ``lines of code'' we follow the definition from \name{SonarQube}.
+LOC = the number of physical lines containing a character which is neither
+whitespace or part of a comment.} and over 25,000 methods, it is a formidable
+task to perform automated changes on it. There should be plenty of situations
+where things can go wrong, and, as we shall see later, they do.
-The configuration for the experiment is specified in
-\myref{tab:configurationCase2}.
+I will start by presenting some statistics from the refactoring execution,
+before I pick apart the \name{SonarQube} analysis and conclude by commenting on
+the results from the unit tests. The configuration for the experiment is
+specified in \myref{tab:configurationCase1}.
\begin{table}[htb]
- \caption{Configuration for Case 2.}
- \label{tab:configurationCase2}
+ \caption{Configuration for Case 1.}
+ \label{tab:configurationCase1}
\centering
\begin{tabularx}{\textwidth}{@{}>{\bfseries}L{0.67}L{1.33}@{}}
\toprule
\spancols{2}{Benchmark data} \\
\midrule
- Launch configuration & CaseStudyDogfooding.launch \\
+ Launch configuration & CaseStudy.launch \\
Project & no.uio.ifi.refaktor.benchmark \\
Repository & gitolite@git.uio.no:ifi-stolz-refaktor \\
Commit & 43c16c04520746edd75f8dc2a1935781d3d9de6c \\
\midrule
\spancols{2}{Input data} \\
\midrule
- Project & no.uio.ifi.refaktor \\
- Repository & gitolite@git.uio.no:ifi-stolz-refaktor \\
- Commit & 43c16c04520746edd75f8dc2a1935781d3d9de6c \\
- Branch & master \\
- Test configuration & no.uio.ifi.refaktor.tests/ExtractTest.launch \\
+ Project & org.eclipse.jdt.ui \\
+ Repository & git://git.eclipse.org/gitroot/jdt/eclipse.jdt.ui.git \\
+ Commit & f218388fea6d4ec1da7ce22432726c244888bb6b \\
+ Branch & R3\_8\_maintenance \\
+ Tests suites & org.eclipse.jdt.ui.tests.AutomatedSuite,
+ org.eclipse.jdt.ui.tests.refactoring.all.\-AllAllRefactoringTests \\
+
\bottomrule
\end{tabularx}
\end{table}
-
\subsection{Statistics}
The statistics gathered during the refactoring execution is presented in
-\myref{tab:case2Statistics}.
+\myref{tab:case1Statistics}.
\begin{table}[htb]
- \caption{Statistics after batch refactoring the \type{no.uio.ifi.refaktor}
-project with the \ExtractAndMoveMethod refactoring.}
- \label{tab:case2Statistics}
+ \caption{Statistics after batch refactoring the Eclipse JDT UI project with
+ the \ExtractAndMoveMethod refactoring.}
+ \label{tab:case1Statistics}
\centering
\begin{tabularx}{\textwidth}{@{}>{\bfseries}L{1.5}R{0.5}@{}}
\toprule
\spancols{2}{Time used} \\
\midrule
- Total time & 1m15s \\
- Analysis time & 0m18s (24\%) \\
- Change time & 0m47s (63\%) \\
- Miscellaneous tasks & 0m10s (14\%) \\
+ Total time & 98m38s \\
+ Analysis time & 14m41s (15\%) \\
+ Change time & 74m20s (75\%) \\
+ Miscellaneous tasks & 9m37s (10\%) \\
\midrule
\spancols{2}{Numbers of each type of entity analyzed} \\
\midrule
- Packages & 33 \\
- Compilation units & 154 \\
- Types & 168 \\
- Methods & 1,070 \\
- Text selections & 8,609 \\
+ Packages & 110 \\
+ Compilation units & 2,097 \\
+ Types & 3,152 \\
+ Methods & 27,667 \\
+ Text selections & 591,500 \\
\midrule
\spancols{2}{Numbers for \ExtractAndMoveMethod refactoring candidates} \\
\midrule
- Methods chosen as candidates & 58 \\
- Methods NOT chosen as candidates & 1,012 \\
- Candidate selections (multiple per method) & 227 \\
+ Methods chosen as candidates & 2,552 \\
+ Methods NOT chosen as candidates & 25,115 \\
+ Candidate selections (multiple per method) & 36,843 \\
\midrule
\spancols{2}{\ExtractAndMoveMethod refactorings executed} \\
\midrule
- Fully executed & 53 \\
- Not fully executed & 5 \\
- Total attempts & 58 \\
+ Fully executed & 2,469 \\
+ Not fully executed & 83 \\
+ Total attempts & 2,552 \\
\midrule
\spancols{2}{Primitive refactorings executed} \\
\spancols{2}{\small \ExtractMethod refactorings} \\
\midrule
- Performed & 56 \\
- Not performed & 2 \\
- Total attempts & 58 \\
+ Performed & 2,483 \\
+ Not performed & 69 \\
+ Total attempts & 2,552 \\
\midrule
\spancols{2}{\small \MoveMethod refactorings} \\
\midrule
- Performed & 53 \\
- Not performed & 3 \\
- Total attempts & 56 \\
+ Performed & 2469 \\
+ Not performed & 14 \\
+ Total attempts & 2,483 \\
\bottomrule
\end{tabularx}
\end{table}
-\subsubsection{Differences}
-There are some differences between the two projects that make them a little
-difficult to compare by performance.
-
-\paragraph{Different complexity.}
-Although the JDT UI project is 39 times greater than the refaktor project in
-terms of lines of code, it is only about 26 times its size measured in numbers
-of methods. This means that the methods in the refaktor project are smaller in
-average than in the JDT project. This is also reflected in the \name{SonarQube}
-report, where the complexity per method for the JDT project is 3.6, while the
-refaktor project has a complexity per method of 2.1.
-
-\paragraph{Number of selections per method.}
-The analysis for the JDT project processed 21 text selections per method in
-average. This number for the refaktor project is only 8 selections per method
-analyzed. This is a direct consequence of smaller methods.
-
-\paragraph{Different candidates to methods ratio.}
-The differences in how the projects are factored are also reflected in the
-ratios for how many methods that are chosen as candidates compared to the total
-number of methods analyzed. For the JDT project, 9\% of the methods were
-considered to be candidates, while for the refaktor project, only 5\% of the
-methods were chosen.
-
-\paragraph{The average number of possible candidate selection.}
-For the methods that are chosen as candidates, the average number of possible
-candidate selections for these methods differ quite much. For the JDT project,
-the number of possible candidate selections for these methods was 14.44
-selections per method, while the candidate methods in the refaktor project had
-only 3.91 candidate selections to choose from, in average.
-
\subsubsection{Execution time}
-The differences in complexity, and the different candidate methods to total
-number of methods ratios, is shown in the distributions of the execution times.
-For the JDT project, 75\% of the total time was used on the actual changes,
-while for the refaktor project, this number was only 63\%.
+I consider the total execution time of approximately 1.5 hours, on a regular
+laptop computer, as being acceptable. It clearly makes the batch process
+unsuitable for doing any on-demand analysis or changes, but it is good enough
+for running periodic jobs, like over-night analysis.
-For the JDT project, the benchmark used on average 0.21 seconds per method in
-the project, while for the refaktor project it used only 0.07 seconds per
-method. So the process used 3 times as much time per method for the JDT project
-than for the refaktor project.
+As the statistics show, 75\% of the total time goes into making the actual code
+changes. The time consumers are here the primitive \ExtractMethod and
+\MoveMethod refactorings. Included in the change time is the parsing and
+precondition checking done by the refactorings, as well as textual changes done
+to files on disk. All this parsing and disk access is time-consuming, and
+constitutes a large part of the change time.
-While the JDT project is 39 times larger than the refaktor project measured in
-lines of code, the benchmark used about 79 times as long time on it than for the
-refaktor project. Relatively, this is about twice as long.
+In comparison, the pure analysis time, used to find suitable candidates, only
+makes up for 15\% of the total time consumed. This includes analyzing almost
+600,000 text selections, while the number of attempted executions of the
+\ExtractAndMoveMethod refactoring is only about 2,500. So the number of executed
+primitive refactorings is approximately 5,000. Assuming the time used on
+miscellaneous tasks are used mostly for parsing source code for the analysis, we
+can say that the time used for analyzing code is at most 25\% of the total time.
+This means that for every primitive refactoring executed, we can analyze around
+360 text selections. So, with an average of about 21 text selections per method,
+ it is reasonable to say that we can analyze over 15 methods in the time it
+ takes to perform a primitive refactoring.
-Since the details of these execution times are not that relevant to this
-master's project, only their magnitude, I will leave them here.
+\subsubsection{Refactoring candidates}
+Out of the 27,667 methods that were analyzed, 2,552 methods contained selections
+that were considered candidates for the \ExtractAndMoveMethod refactoring. This
+is roughly 9\% off the methods in the project. These 9\% of the methods had on
+average 14.4 text selections that were considered possible refactoring
+candidates.
\subsubsection{Executed refactorings}
-For the composite \ExtractAndMoveMethod refactoring performed in case 2, 53
-successful attempts out of 58 gives a success rate of 91.4\%. This is 5.3
-percentage points worse than for case 1.
+2,469 out of 2,552 attempts on executing the \ExtractAndMoveMethod refactoring
+were successful, giving a success rate of 96.7\%. The failure rate of 3.3\%
+stems from situations where the analysis finds a candidate selection, but the
+change execution fails. This failure could be an exception that was thrown, and
+the refactoring aborts. It could also be the precondition checking for one of
+the primitive refactorings that gives us an error status, meaning that if the
+refactoring proceeds, the code will contain compilation errors afterwards,
+forcing the composite refactoring to abort. This means that if the
+\ExtractMethod refactoring fails, no attempt is done for the \MoveMethod
+refactoring. \todo{Redundant information? Put in benchmark chapter?}
-\subsection{\name{SonarQube} analysis}
-Results from the \name{SonarQube} analysis is shown in
-\myref{tab:case2ResultsProfile1}.
+Out of the 2,552 \ExtractMethod refactorings that were attempted executed, 69 of
+them failed. This gives a failure rate of 2.7\% for the primitive refactoring.
+In comparison, the \MoveMethod refactoring had a failure rate of 0.6 \% of the
+2,483 attempts on the refactoring.
-Not much is to be said about these results. The trends in complexity and
-coupling are the same. We end up a little worse after the refactoring process
-than before.
+The failure rates for the refactorings are not that bad, if we also take into
+account that the pre-refactoring analysis is incomplete.\todo{see \ldots}
+
+\subsection{\name{SonarQube} analysis}
+Results from the \name{SonarQube} analysis are shown in
+\myref{tab:case1ResultsProfile1}.
\begin{table}[htb]
- \caption{Results for analyzing the \var{no.uio.ifi.refaktor} project, before
- and after the refactoring, with \name{SonarQube} and the \name{IFI Refaktor
- Case Study} quality profile. (Bold numbers are better.)}
- \label{tab:case2ResultsProfile1}
+ \caption{Results for analyzing the Eclipse JDT UI project, before and after
+ the refactoring, with \name{SonarQube} and the \name{IFI Refaktor Case Study}
+ quality profile. (Bold numbers are better.)}
+ \label{tab:case1ResultsProfile1}
\centering
\begin{tabularx}{\textwidth}{@{}>{\bfseries}L{1.5}R{0.25}R{0.25}@{}}
\toprule
\textnormal{Number of issues for each rule} & Before & After \\
\midrule
- Avoid too complex class & 1 & 1 \\
+ Avoid too complex class & 81 & \textbf{79} \\
Classes should not be coupled to too many other classes (Single
- Responsibility Principle) & \textbf{29} & 34 \\
- Control flow statements \ldots{} should not be nested too deeply & 24 &
- \textbf{21} \\
- Methods should not be too complex & 17 & \textbf{15} \\
- Methods should not have too many lines & 41 & \textbf{40} \\
- NPath Complexity & 3 & 3 \\
- Too many methods & \textbf{13} & 15 \\
+ Responsibility Principle) & \textbf{1,098} & 1,199 \\
+ Control flow statements \ldots{} should not be nested too deeply & 1,375 &
+ \textbf{1,285} \\
+ Methods should not be too complex & 1,518 & \textbf{1,452} \\
+ Methods should not have too many lines & 3,396 & \textbf{3,291} \\
+ NPath Complexity & 348 & \textbf{329} \\
+ Too many methods & \textbf{454} & 520 \\
\midrule
- Total number of issues & \textbf{128} & 129 \\
+ Total number of issues & 8,270 & \textbf{8,155} \\
\midrule
\midrule
\spancols{3}{Complexity} \\
\midrule
- Per function & 2.1 & 2.1 \\
- Per class & \textbf{12.5} & 12.9 \\
- Per file & \textbf{13.8} & 14.2 \\
+ Per function & 3.6 & \textbf{3.3} \\
+ Per class & \textbf{29.5} & 30.4 \\
+ Per file & \textbf{44.0} & 45.3 \\
\midrule
- Total complexity & \textbf{2,089} & 2,148 \\
+ Total complexity & \textbf{84,765} & 87,257 \\
\midrule
\midrule
\spancols{3}{Numbers of each type of entity analyzed} \\
\midrule
- Files & 151 & 151 \\
- Classes & 167 & 167 \\
- Functions & 987 & 1,045 \\
- Accessors & 35 & 30 \\
- Statements & 3,355 & 3,416 \\
- Lines of code & 8,238 & 8,460 \\
+ Files & 1,926 & 1,926 \\
+ Classes & 2,875 & 2,875 \\
+ Functions & 23,744 & 26,332 \\
+ Accessors & 1,296 & 1,019 \\
+ Statements & 162,768 & 165,145 \\
+ Lines of code & 320,941 & 329,112 \\
\midrule
- Technical debt (in days) & \textbf{19.0} & 20.7 \\
+ Technical debt (in days) & \textbf{1,003.4} & 1,032.7 \\
\bottomrule
\end{tabularx}
\end{table}
-\subsection{Unit tests}
-The tests used for this case are the same that has been developed throughout the
-master's project.
-
-The code that was refactored for this case suffered from some of the problems
-discovered in case 1. This means that the after-code for case 2 also contained
-compilation errors, but they were not as many. The code contained only 6 errors
-that made the code not compile.
-
-All of the errors made, originated from the same bug. It is a bug that happens
-in situation where a class instance creation is moved from between packages, and
-the class for the instance is package-private. The \MoveMethod refactoring does
-not detect that there will be a visibility problem, and neither does it promote
-the package-private class to be public.
-
-Since the errors were easy to fix manually, I corrected them and ran the unit
-tests as planned. Before the refactoring, all tests passed. All tests also
-passed after the refactoring, with the six error corrections. Since the
-corrections done is not of a kind that could make the behavior of the program
-change, it is likely that the refactorings done to the
-\type{no.uio.ifi.refaktor} project did not change its behavior. This is also
-supported by the informal experiment presented next.
-
-\subsection{An informal experiment}
-To complete the task of ``eating my own dog food'', I conducted an informal
-experiment where I used the refactored version of the \type{no.uio.ifi.refaktor}
-project, with the corrections, to again refaktor ``itself''.
-
-The experiment produced code containing the same six errors as after the
-previous experiment. I also compared the after-code from the two experiments
-with a diff-tool. The only differences found were different method names. This
-is expected, since the method names are randomly generated by the
-\ExtractAndMoveMethod refactoring.
-
-The outcome of this simple experiment makes me more confident that the
-\ExtractAndMoveMethod refactoring made only behavior-preserving changes to the
-\type{no.uio.ifi.refaktor} project, apart from the compilation errors.
-
-\subsection{Conclusions}
-The differences in complexity between the Eclipse JDT UI project and the
-\type{no.uio.ifi.refaktor} project, clearly influenced the differences in their
-execution times. This is mostly because fewer of the methods were chosen to be
-refactored for the refaktor project than for the JDT project. What this makes
-difficult, is to know if there are any severe performance penalties associated
-with refactoring on a large project compared to a small one.
-
-The trends in the \name{SonarQube} analysis are the same for this case as for
-the previous one. This gives more confidence in the these results.
-
-By refactoring our own code and using it again to refactor our code, we showed
-that it is possible to write an automated composite refactoring that works for
-many cases. That it probably did not alter the behavior of a smaller project
-shows us nothing more than that though, and might just be a coincidence.
-
-\section{Summary}
-\todoin{Write? Or wrap up in final conclusions?}
-
-\chapter{Benchmarking}\label{sec:benchmarking}
-This part of the master's project is located in the \name{Eclipse} project
-\code{no.uio.ifi.refaktor.benchmark}. The purpose of it is to run the equivalent
-of the \type{SearchBasedExtractAndMoveMethodChanger}
-\see{searchBasedExtractAndMoveMethodChanger} over a larger software project,
-both to test its robustness but also its effect on different software metrics.
-
-\section{The benchmark setup}
-The benchmark itself is set up as a \name{JUnit} test case. This is a convenient
-setup, and utilizes the \name{JUnit Plugin Test Launcher}. This provides us with
-a fully functional \name{Eclipse} workbench. Most importantly, this gives us
-access to the Java Model of \name{Eclipse} \see{javaModel}.
-
-\subsection{The ProjectImporter}
-The Java project that is going to be used as the data for the benchmark, must be
-imported into the JUnit workspace. This is done by the
-\typewithref{no.uio.ifi.refaktor.benchmark}{ProjectImporter}. The importer
-require the absolute path to the project description file. It is named
-\code{.project} and is located at the root of the project directory.
-
-The project description is loaded to find the name of the project to be
-imported. The project that shall be the destination for the import is created in
-the workspace, on the base of the name from the description. Then an import
-operation is created, based on both the source and destination information. The
-import operation is run to perform the import.
-
-I have found no simple API call to accomplish what the importer does, which
-tells me that it may not be too many people performing this particular action.
-The solution to the problem was found on \name{Stack
-Overflow}\footnote{\url{https://stackoverflow.com/questions/12401297}}. It
-contains enough dirty details to be considered inconvenient to use, if not
-wrapping it in a class like my \type{ProjectImporter}. One would probably have
-to delve into the source code for the import wizard to find out how the import
-operation works, if no one had already done it.
-
-\section{Statistics}
-Statistics for the analysis and changes is captured by the
-\typewithref{no.uio.ifi.refaktor.aspects}{StatisticsAspect}. This an
-\emph{aspect} written in \name{AspectJ}.
-
-\subsection{AspectJ}
-\name{AspectJ}\footnote{\url{http://eclipse.org/aspectj/}} is an extension to
-the Java language, and facilitates combining aspect-oriented programming with
-the object-oriented programming in Java.
-
-Aspect-oriented programming is a programming paradigm that is meant to isolate
-so-called \emph{cross-cutting concerns} into their own modules. These
-cross-cutting concerns are functionalities that spans over multiple classes, but
-may not belong naturally in any of them. It can be functionality that does not
-concern the business logic of an application, and thus may be a burden when
-entangled with parts of the source code it does not really belong. Examples
-include logging, debugging, optimization and security.
-
-Aspects are interacting with other modules by defining advices. The concept of
-an \emph{advice} is known from both aspect-oriented and functional
-programming\citing{wikiAdvice2014}. It is a function that modifies another
-function when the latter is run. An advice in AspectJ is somewhat similar to a
-method in Java. It is meant to alter the behavior of other methods, and contains
-a body that is executed when it is applied.
-
-An advice can be applied at a defined \emph{pointcut}. A pointcut picks out one
-or more \emph{join points}. A join point is a well-defined point in the
-execution of a program. It can occur when calling a method defined for a
-particular class, when calling all methods with the same name,
-accessing/assigning to a particular field of a given class and so on. An advice
-can be declared to run both before, after returning from a pointcut, when there
-is thrown an exception in the pointcut or after the pointcut either returns or
-throws an exception. In addition to picking out join points, a pointcut can
-also bind variables from its context, so they can be accessed in the body of an
-advice. An example of a pointcut and an advice is found in
-\myref{lst:aspectjExample}.
-
-\begin{listing}[h]
-\begin{minted}{aspectj}
-pointcut methodAnalyze(
- SearchBasedExtractAndMoveMethodAnalyzer analyzer) :
- call(* SearchBasedExtractAndMoveMethodAnalyzer.analyze())
- && target(analyzer);
-
-after(SearchBasedExtractAndMoveMethodAnalyzer analyzer) :
- methodAnalyze(analyzer) {
- statistics.methodCount++;
- debugPrintMethodAnalysisProgress(analyzer.method);
-}
-\end{minted}
-\caption{An example of a pointcut named \method{methodAnalyze},
-and an advice defined to be applied after it has occurred.}
-\label{lst:aspectjExample}
-\end{listing}
-
-\subsection{The Statistics class}
-The statistics aspect stores statistical information in an object of type
-\type{Statistics}. As of now, the aspect needs to be initialized at the point in
-time where it is desired that it starts its data gathering. At any point in time
-the statistics aspect can be queried for a snapshot of the current statistics.
-
-The \type{Statistics} class also include functionality for generating a report
-of its gathered statistics. The report can be given either as a string or it can
-be written to a file.
+\subsubsection{Diversity in the number of entities analyzed}
+The analysis performed by \name{SonarQube} is reporting fewer methods than found
+by the pre-refactoring analysis. \name{SonarQube} discriminates between
+functions (methods) and accessors, so the 1,296 accessors play a part in this
+calculation. \name{SonarQube} also has the same definition as our plugin when
+it comes to how a class is defined. Therefore it seems like \name{SonarQube}
+misses 277 classes that our plugin handles. This can explain why the {SonarQube}
+report differs from our numbers by approximately 2,500 methods,
-\subsection{Advices}
-The statistics aspect contains advices for gathering statistical data from
-different parts of the benchmarking process. It captures statistics from both
-the analysis part and the execution part of the composite \ExtractAndMoveMethod
-refactoring.
+\subsubsection{Complexity}
+On all complexity rules that works on the method level, the number of issues
+decreases with between 3.1\% and 6.5\% from before to after the refactoring. The
+average complexity of a method decreases from 3.6 to 3.3, which is an
+improvement of about 8.3\%. So, on the method level, the refactoring must be
+said to have a slightly positive impact. This is due to the extraction of a lot
+of methods, making the average method size smaller.
-For the analysis part, there are advices to count the number of text selections
-analyzed and the number of methods, types, compilation units and packages
-analyzed. There are also advices that counts for how many of the methods there
-is found a selection that is a candidate for the refactoring, and for how many
-methods there is not.
+The improvement in complexity on the method level is somewhat traded for
+complexity on the class level. The complexity per class metric is worsened by
+3\% from before to after. The issues for the ``Too many methods'' rule also
+increases by 14.5\%. These numbers indicate that the refactoring makes quite a
+lot of the classes a little more complex overall. This is the expected outcome,
+since the \ExtractAndMoveMethod refactoring introduces almost 2,500 new methods
+into the project.
-There exists advices for counting both the successful and unsuccessful
-executions of all the refactorings. Both for the \ExtractMethod and \MoveMethod
-refactorings in isolation, as well as for the combination of them.
+The only number that can save the refactoring's impact on complexity on the
+class level, is the ``Avoid too complex class'' rule. It improves with 2.5\%,
+thus indicating that the complexity is moderately better distributed between the
+classes after the refactoring than before.
-\section{Optimizations}
-When looking for optimizations to make for the benchmarking process, I used the
-\name{VisualVM}\footnote{\url{http://visualvm.java.net/}} \gloss{profiler} for
-the Java Virtual Machine to both profile the application and also to make memory
-dumps of its heap.
+\subsubsection{Coupling}
+One of the hopes when starting this project, was to be able to make a
+refactoring that could lower the coupling between classes. Better complexity at
+the method level is a not very unexpected byproduct of dividing methods into
+smaller parts. Lowering the coupling on the other hand, is a far greater task.
+This is also reflected in the results for the only coupling rule defined in the
+\name{SonarQube} quality profile, namely the ``Classes should not be coupled to
+too many
+other classes (Single Responsibility Principle)'' rule.
-\subsection{Caching}
-When \gloss{profiling} the benchmark process before making any optimizations, it
-early became apparent that the parsing of source code was a place to direct
-attention towards. This discovery was done when only \emph{analyzing} source
-code, before trying to do any \emph{manipulation} of it. Caching of the parsed
-ASTs seemed like the best way to save some time, as expected. With only a simple
-cache of the most recently used AST, the analysis time was speeded up by a
-factor of around 20. This number depends a little upon which type of system the
-analysis is run.
+The number of issues for the coupling rule is 1,098 before the refactoring, and
+1,199 afterwards. This is an increase in issues of 9.2\%. These numbers can be
+interpreted two ways. The first possibility is that our assumptions are wrong,
+and that increasing indirection does not decrease coupling between classes. The
+other possibility is that our analysis and choices of candidate text selections
+are not good enough. I vote for the second possibility. (Voting against the
+public opinion may also be a little bold.)
+
+\subsubsection{An example of what makes the number of dependency issues grow}
+\Myref{lst:sonarJDTExampleBefore} shows a portion of the class
+\typewithref{org.eclipse.jdt.ui.actions}{ShowActionGroup} from the JDT UI
+project before it is refactored with the search-based \ExtractAndMoveMethod
+refactoring. Before the refactoring, the \type{ShowActionGroup} class has 12
+outgoing dependencies (reported by \name{SonarQube}).
-The caching is managed by a cache manager, that now, by default, utilizes the
-not so well known feature of Java called a \emph{soft reference}. Soft
-references are best explained in the context of weak references. A \emph{weak
-reference} is a reference to an object instance that is only guaranteed to
-persist as long as there is a \emph{strong reference} or a soft reference
-referring the same object. If no such reference is found, its referred object is
-garbage collected. A strong reference is basically the same as a regular Java
-reference. A soft reference has the same guarantees as a week reference when it
-comes to its relation to strong references, but it is not necessarily garbage
-collected whenever there exists no strong references to it. A soft reference
-\emph{may} reside in memory as long as the JVM has enough free memory in the
-heap. A soft reference will therefore usually perform better than a weak
-reference when used for simple caching and similar tasks. The way to use a
-soft/weak reference is to as it for its referent. The return value then has to
-be tested to check that it is not \var{null}. For the basic usage of soft
-references, see \myref{lst:softReferenceExample}. For a more thorough
-explanation of weak references in general, see\citing{weakRef2006}.
+\begin{listing}[htb]
+\begin{minted}[linenos,samepage]{java}
+public class ShowActionGroup extends ActionGroup {
+ /* ... */
+ private void initialize(IWorkbenchSite site,
+ boolean isJavaEditor) {
+ fSite= site;
+ ISelectionProvider provider= fSite.getSelectionProvider();
+ ISelection selection= provider.getSelection();
+ fShowInPackagesViewAction.update(selection);
+ if (!isJavaEditor) {
+ provider.addSelectionChangedListener(
+ fShowInPackagesViewAction);
+ }
+ }
+}
+\end{minted}
+\caption{Portion of the \type{ShowActionGroup} class before refactoring.}
+\label{lst:sonarJDTExampleBefore}
+\end{listing}
-\begin{listing}[h]
-\begin{minted}{java}
-// Strong reference
-Object strongRef = new Object();
+During the benchmark process, the search-based \ExtractAndMoveMethod refactoring
+extracts the lines 6 to 12 of the code in \myref{lst:sonarJDTExampleBefore}, and
+moves the new method to the move target, which is the field
+\var{fShowInPackagesViewAction} with type
+\typewithref{org.eclipse.jdt.ui.actions}{ShowInPackageViewAction}. The result is
+shown in \myref{lst:sonarJDTExampleAfter}.
-// Soft reference
-SoftReference<Object> softRef =
- new SoftReference<Object>(new Object());
+\begin{listing}[htb]
+\begin{minted}[linenos,samepage]{java}
+public class ShowActionGroup extends ActionGroup {
+ /* ... */
+ private void initialize(IWorkbenchSite site,
+ boolean isJavaEditor) {
+ fSite= site;
+ fShowInPackagesViewAction.generated_8019497110545412081(
+ this, isJavaEditor);
+ }
+}
+\end{minted}
-// Using the soft reference
-Object obj = softRef.get();
-if (obj != null) {
- // Use object here
+\begin{minted}[linenos,samepage]{java}
+public class ShowInPackageViewAction
+ extends SelectionDispatchAction {
+ /* ... */
+ public void generated_8019497110545412081(
+ ShowActionGroup showactiongroup, boolean isJavaEditor) {
+ ISelectionProvider provider=
+ showactiongroup.fSite.getSelectionProvider();
+ ISelection selection= provider.getSelection();
+ update(selection);
+ if (!isJavaEditor) {
+ provider.addSelectionChangedListener(this);
+ }
+ }
}
\end{minted}
-\caption{Showing the basic usage of soft references. Weak references is used the
- same way. {\footnotesize (The references are part of the \code{java.lang.ref}
-package.)}}
-\label{lst:softReferenceExample}
+\caption{Portions of the classes \type{ShowActionGroup} and
+\type{ShowInPackageViewAction} after refactoring.}
+\label{lst:sonarJDTExampleAfter}
\end{listing}
-The cache based on soft references has no limit for how many ASTs it caches. It
-is generally not advisable to keep references to ASTs for prolonged periods of
-time, since they are expensive structures to hold on to. For regular plugin
-development, \name{Eclipse} recommends not creating more than one AST at a time to
-limit memory consumption. Since the benchmarking has nothing to do with user
-experience, and throughput is everything, these advices are intentionally
-ignored. This means that during the benchmarking process, the target \name{Eclipse}
-application may very well work close to its memory limit for the heap space for
-long periods during the benchmark.
+After the refactoring, the \type{ShowActionGroup} has only 11 outgoing
+dependencies. It no longer depends on the
+\typewithref{org.eclipse.jface.viewers}{ISelection} type. So our refactoring
+managed to get rid of one dependency, which is exactly what we wanted. The only
+problem is, that now the \type{ShowInPackageViewAction} class has got two new
+dependencies, in the \type{ISelectionProvider} and the \type{ISelection} types.
+The bottom line is that we eliminated one dependency, but introduced two more,
+ending up with a program that has more dependencies now than when we started.
+
+What can happen in many situations where the \ExtractAndMoveMethod refactoring
+is performed, is that the \MoveMethod refactoring ``drags'' with it references
+to classes that are unknown to the method destination. If the refactoring
+happens to be so lucky that it removes a dependency from one class, it might as
+well introduce a couple of new dependencies to another class, as shown in the
+previous example. In those situations where a destination class does not know
+about the originating class of a moved method, the \MoveMethod refactoring most
+certainly will introduce a dependency. This is because there is a
+bug\footnote{\href{https://bugs.eclipse.org/bugs/show\_bug.cgi?id=228635}{Eclipse
+Bug 228635 - [move method] unnecessary reference to source}} in the refactoring,
+making it pass an instance of the originating class as a reference to the moved
+method, regardless of whether the reference is used in the method body or not.
-\subsection{Candidates stored as mementos}
-When performing large scale analysis of source code for finding candidates to
-the \ExtractAndMoveMethod refactoring, memory is an issue. One of the inputs to
-the refactoring is a variable binding. This variable binding indirectly retains
-a whole AST. Since ASTs are large structures, this quickly leads to an
-\type{OutOfMemoryError} if trying to analyze a large project without optimizing
-how we store the candidates data. This means that the JVM cannot allocate more
-memory for out benchmark, and it exists disgracefully.
+There is also the possibility that the heuristics used to find candidate text
+selections are not good enough. There is work to be done with fine-tuning the
+heuristics and to complete the analysis part of this project.
-A possible solution could be to just allow the JVM to allocate even more memory,
-but this is not a dependable solution. The allocated memory could easily
-supersede the physical memory of a machine, and that would make the benchmark go
-really slow.
+\subsubsection{Totals}
+On the bright side, the total number of issues is lower after the refactoring
+than it was before. Before the refactoring, the total number of issues was
+8,270, and after it is 8,155. This is an improvement of 1.4\%.
-Thus, the candidates data must be stored in another format. Therefore, we use
-the \gloss{mementoPattern} to store the variable binding information. This is
-done in a way that makes it possible to retrieve the variable binding at a later
-point. The data that is stored to achieve this, is the key to the original
-variable binding. In addition to the key, we know which method and text
-selection the variable is referenced in, so that we can find it by parsing the
-source code and search for it when it is needed.
+The down side is that \name{SonarQube} shows that the total cyclomatic
+complexity has increased by 2.9\%, and that the (more questionable) ``technical
+debt'' has increased from 1,003.4 to 1,032.7 days, also a deterioration of
+2.9\%. Although these numbers are similar, no correlation has been found
+between them.
-\section{Handling failures}
-\todoin{write}
+\subsection{Unit tests}
+The tests that have been run for the \name{Eclipse JDT UI} project, are the
+test suites specified as the main test suites on the JDT UI wiki page on how to
+contribute to the
+project\footnote{\url{https://wiki.eclipse.org/JDT\_UI/How\_to\_Contribute\#Unit\_Testing}}.
+The results from these tests are shown in \myref{tab:case1UnitTests}.
+\begin{table}[htb]
+ \caption{Results from the unit tests run for the Eclipse JDT UI project,
+ before and after the refactoring.}
+ \label{tab:case1UnitTests}
+ \centering
+ \begin{tabularx}{\textwidth}{@{}>{\bfseries}L{1}R{0.5}R{0.5}@{}}
+ \toprule
+ \textnormal{AutomatedSuite} & Before & After \\
+ \midrule
+ Runs & 2007/2007 & 2007/2007 \\
+ Errors & 4 & 565 \\
+ Failures & 3 & 5 \\
+ \midrule
+ \spancols{2}{AllAllRefactoringTests} \\
+ \midrule
+ Runs & 3815/3816 & 3815/3816 \\
+ Errors & 2 & 2257 \\
+ Failures & 3 & 0 \\
+ \bottomrule
+ \end{tabularx}
+\end{table}
-\chapter{Technicalities}
+\subsubsection{Before the refactoring}
+Running the tests for the before-code of Eclipse JDT UI yielded 4 errors and 3
+failures for the \type{AutomatedSuite} test suite (2,007 test cases), and 2
+errors and 3 failures for the \type{AllAllRefactoringTests} test suite (3,816
+test cases).
-\section{Source code organization}
-All the parts of this master's project is under version control with
-\name{Git}\footnote{\url{http://git-scm.com/}}.
+\subsubsection{After the refactoring}
+For the after-code of the Eclipse JDT UI project, Eclipse reports that the
+project contains 322 compilation errors, and a lot of other errors that
+follow from these. All of the errors are caused by the \ExtractAndMoveMethod
+refactoring. Had these errors originated from only one bug, it would not have
+been much of a problem, but this is not the case. By only looking at some random
+compilation problems in the refactored code, I came up with at least four
+different bugs \todo{write bug reports?} that caused those problems. I then
+stopped looking for more, since some of the bugs would take more time to fix
+than I could justify using on them at this point.
+
+One thing that can be said in my defense, is that all the compilation errors
+could have been avoided if the types of situations that cause them were properly
+handled by the primitive refactorings, which again are supplied by the Eclipse
+JDT UI project. All four bugs that I mentioned before are weaknesses of the
+\MoveMethod refactoring. If the primitive refactorings had detected the
+up-coming errors in their precondition checking phase, the refactorings would
+have been aborted, since this is how the \ExtractAndMoveMethod refactoring
+handles such situations. This shows that it is not safe to completely rely upon
+the primitive refactorings to save us if our own pre-refactoring analysis fails
+to detect that a compilation error will be introduced. A problem is that the
+source code analysis done by both the JDT refactorings and our own tool is
+incomplete.
+
+Of course, taking into account all possible situations that could lead to
+compilation errors is an immense task. A complete analysis of these situations
+is too big of a problem for this master's project to solve. Looking at it now,
+this comes as no surprise, since the task is obviously also too big for the
+creators of the primitive \MoveMethod refactoring.
-The software written is organized as some \name{Eclipse} plugins. Writing a plugin is
-the natural way to utilize the API of \name{Eclipse}. This also makes it possible to
-provide a user interface to manually run operations on selections in program
-source code or whole projects/packages.
+Considering all these problems, it is difficult to know how to interpret the
+unit test results from after refactoring the Eclipse JDT UI. The
+\type{AutomatedSuite} reported 565 errors and 5 failures, which means that 1437,
+or 71.6\%, of the tests still passed. Three of the failures were the same as
+reported before the refactoring took place, so two of them are new. For these
+two cases it is not immediately apparent what makes them behave differently. The
+program is so complex that to analyze it to find this out, we might need more
+powerful methods than just manually analyzing its source code. This is somewhat
+characteristic for imperative programming: The programs are often hard to
+analyze and understand.
-When writing a plugin in \name{Eclipse}, one has access to resources such as the
-current workspace, the open editor and the current selection.
+For the \type{AllAllRefactoringTests} test suite, the three failures are gone,
+but the two errors have grown to 2,257 errors. I will not try to analyze those
+errors.
-The thesis work is contained in the following Eclipse projects:
+What I can say at this point, is that it is likely that the
+\ExtractAndMoveMethod refactoring has introduced some unintentional behavioral
+changes. Let us say that the refactoring introduces at least two
+behavior-altering changes for every 2,500 executions. More than that is
+difficult to say about the behavior-preserving properties of the
+\ExtractAndMoveMethod refactoring, at this point.
-\begin{description}
- \item[no.uio.ifi.refaktor] \hfill \\ This is the main Eclipse plugin
- project, and contains all of the business logic for the plugin.
+\subsection{Conclusions}
+After automatically analyzing and executing the \ExtractAndMoveMethod
+refactoring for all the methods in the Eclipse JDT UI project, the results do
+not look that promising. For this case, the refactoring seems almost unusable as
+it is now. The error rate and measurements tell us this.
- \item[no.uio.ifi.refaktor.tests] \hfill \\
- This project contains the tests for the main plugin.
+The refactoring makes the code a little less complex at the method level. But
+this is merely a side effect of extracting methods. When it comes to the overall
+complexity, it is increased, although it is slightly better spread among the
+classes.
- \item[no.uio.ifi.refaktor.examples] \hfill \\
- Contains example code used in testing. It also contains code for managing
- this example code, such as creating an Eclipse project from it before a test
- run.
+The analysis done before the \ExtractAndMoveMethod refactoring, is currently not
+complete enough to make the refactoring useful. It introduces too many errors in
+the code, and the code may change its behavior. It also remains to prove that
+large scale refactoring with it can decrease coupling between classes. A better
+analysis may prove this, but in its present state, the opposite is the fact. The
+coupling measurements done by \name{SonarQube} show this.
- \item[no.uio.ifi.refaktor.benchmark] \hfill \\
- This project contains code for running search based versions of the
- composite refactoring over selected Eclipse projects.
+On the bright side, the performance of the refactoring process is not that bad.
+It shows that it is possible to make a tool the way we do, if we can make the
+tool do anything useful. As long as the analysis phase is not going to involve
+anything that uses too much disk access, a lot of analysis can be done in a
+reasonable amount of time.
- \item[no.uio.ifi.refaktor.releng] \hfill \\
- Contains the rmap, queries and target definitions needed by by Buckminster
- on the Jenkins continuous integration server.
+The time used on performing the actual changes excludes a trial and error
+approach with the tools used in this master's project. In a trial and error
+approach, you could for instance be using the primitive refactorings used in
+this project to refactor code, and only then make decisions based on the effect,
+possibly shown by traditional software metrics. The problem with the approach
+taken in this project, compared to a trial and error approach, is that using
+heuristics beforehand is much more complicated. But on the other hand, a trial
+and error approach would still need to face the challenges of producing code
+that does compile without errors. If using refactorings that could produce
+in-memory changes, a trial and error approach could be made more efficient.
-\end{description}
+\section{Case 2: The \type{no.uio.ifi.refaktor} project}
+In this case we will see a form of the ``dogfooding'' methodology used, when
+refactoring our own \type{no.uio.ifi.refaktor} project with the
+\ExtractAndMoveMethod refactoring.
-\subsection{The no.uio.ifi.refaktor project}
+In this case I will try to point out some differences from the first case, and
+how they impact the execution of the benchmark. The refaktor project is 39 times
+smaller than the Eclipse JDT UI project, measured in lines of code. This will
+make things a bit more transparent. It will therefore be interesting to see if
+this case can shed light on any aspect of our project that were lost in the
+larger case 1.
-\subsubsection{no.uio.ifi.refaktor.analyze}
-This package, and its sub-packages, contains code that is used for analyzing
-Java source code. The most important sub-packages are presented below.
+The configuration for the experiment is specified in
+\myref{tab:configurationCase2}.
-\begin{description}
- \item[no.uio.ifi.refaktor.analyze.analyzers] \hfill \\
- This package contains source code analyzers. These are usually responsible
- for analyzing text selections or running specialized analyzers for different
- kinds of entities. Their structure are often hierarchical. This means that
- you have an analyzer for text selections, that in turn is utilized by an
- analyzer that analyzes all the selections of a method. Then there are
- analyzers for analyzing all the methods of a type, all the types of a
- compilation unit, all the compilation units of a package, and, at last, all
- of the packages in a project.
+\begin{table}[htb]
+ \caption{Configuration for Case 2.}
+ \label{tab:configurationCase2}
+ \centering
+ \begin{tabularx}{\textwidth}{@{}>{\bfseries}L{0.67}L{1.33}@{}}
+ \toprule
+ \spancols{2}{Benchmark data} \\
+ \midrule
+ Launch configuration & CaseStudyDogfooding.launch \\
+ Project & no.uio.ifi.refaktor.benchmark \\
+ Repository & gitolite@git.uio.no:ifi-stolz-refaktor \\
+ Commit & 43c16c04520746edd75f8dc2a1935781d3d9de6c \\
+ \midrule
+ \spancols{2}{Input data} \\
+ \midrule
+ Project & no.uio.ifi.refaktor \\
+ Repository & gitolite@git.uio.no:ifi-stolz-refaktor \\
+ Commit & 43c16c04520746edd75f8dc2a1935781d3d9de6c \\
+ Branch & master \\
+ Test configuration & no.uio.ifi.refaktor.tests/ExtractTest.launch \\
+ \bottomrule
+ \end{tabularx}
+\end{table}
- \item[no.uio.ifi.refaktor.analyze.checkers] \hfill \\
- A package containing checkers. The checkers are classes used to validate
- that a selection can be further analyzed and chosen as a candidate for a
- refactoring. Invalidating properties can be such as usage of inner classes
- or the need for multiple return values.
+\subsection{Statistics}
+The statistics gathered during the refactoring execution is presented in
+\myref{tab:case2Statistics}.
- \item[no.uio.ifi.refaktor.analyze.collectors] \hfill \\
- This package contains the property collectors. Collectors are used to gather
- properties from a text selection. This is mostly properties regarding
- referenced names and their occurrences. It is these properties that makes up
- the basis for finding the best candidates for a refactoring.
-\end{description}
+\begin{table}[htb]
+ \caption{Statistics after batch refactoring the \type{no.uio.ifi.refaktor}
+project with the \ExtractAndMoveMethod refactoring.}
+ \label{tab:case2Statistics}
+ \centering
+ \begin{tabularx}{\textwidth}{@{}>{\bfseries}L{1.5}R{0.5}@{}}
+ \toprule
+ \spancols{2}{Time used} \\
+ \midrule
+ Total time & 1m15s \\
+ Analysis time & 0m18s (24\%) \\
+ Change time & 0m47s (63\%) \\
+ Miscellaneous tasks & 0m10s (14\%) \\
+ \midrule
+ \spancols{2}{Numbers of each type of entity analyzed} \\
+ \midrule
+ Packages & 33 \\
+ Compilation units & 154 \\
+ Types & 168 \\
+ Methods & 1,070 \\
+ Text selections & 8,609 \\
+ \midrule
+ \spancols{2}{Numbers for \ExtractAndMoveMethod refactoring candidates} \\
+ \midrule
+ Methods chosen as candidates & 58 \\
+ Methods NOT chosen as candidates & 1,012 \\
+ Candidate selections (multiple per method) & 227 \\
+ \midrule
+ \spancols{2}{\ExtractAndMoveMethod refactorings executed} \\
+ \midrule
+ Fully executed & 53 \\
+ Not fully executed & 5 \\
+ Total attempts & 58 \\
+ \midrule
+ \spancols{2}{Primitive refactorings executed} \\
+ \spancols{2}{\small \ExtractMethod refactorings} \\
+ \midrule
+ Performed & 56 \\
+ Not performed & 2 \\
+ Total attempts & 58 \\
+ \midrule
+ \spancols{2}{\small \MoveMethod refactorings} \\
+ \midrule
+ Performed & 53 \\
+ Not performed & 3 \\
+ Total attempts & 56 \\
-\subsubsection{no.uio.ifi.refaktor.change}
-This package, and its sub-packages, contains functionality for manipulate source
-code.
+ \bottomrule
+ \end{tabularx}
+\end{table}
-\begin{description}
- \item[no.uio.ifi.refaktor.change.changers] \hfill \\
- This package contains source code changers. They are used to glue together
- the analysis of source code and the actual execution of the changes.
+\subsubsection{Differences}
+There are some differences between the two projects that make them a little
+difficult to compare by performance.
- \item[no.uio.ifi.refaktor.change.executors] \hfill \\
- The executors that are responsible for making concrete changes are found in
- this package. They are mostly used to create and execute one or more Eclipse
- refactorings.
+\paragraph{Different complexity.}
+Although the JDT UI project is 39 times greater than the refaktor project in
+terms of lines of code, it is only about 26 times its size measured in numbers
+of methods. This means that the methods in the refaktor project are smaller in
+average than in the JDT project. This is also reflected in the \name{SonarQube}
+report, where the complexity per method for the JDT project is 3.6, while the
+refaktor project has a complexity per method of 2.1.
- \item[no.uio.ifi.refaktor.change.processors] \hfill \\
- Contains a refactoring processor for the \MoveMethod refactoring. The code
- is stolen and modified to fix a bug. The related bug is described in
- \myref{eclipse_bug_429416}.
+\paragraph{Number of selections per method.}
+The analysis for the JDT project processed 21 text selections per method in
+average. This number for the refaktor project is only 8 selections per method
+analyzed. This is a direct consequence of smaller methods.
-\end{description}
+\paragraph{Different candidates to methods ratio.}
+The differences in how the projects are factored are also reflected in the
+ratios for how many methods that are chosen as candidates compared to the total
+number of methods analyzed. For the JDT project, 9\% of the methods were
+considered to be candidates, while for the refaktor project, only 5\% of the
+methods were chosen.
-\subsubsection{no.uio.ifi.refaktor.handlers}
-This package contains handlers for the commands defined in the plugin manifest.
+\paragraph{The average number of possible candidate selection.}
+For the methods that are chosen as candidates, the average number of possible
+candidate selections for these methods differ quite much. For the JDT project,
+the number of possible candidate selections for these methods was 14.44
+selections per method, while the candidate methods in the refaktor project had
+only 3.91 candidate selections to choose from, in average.
-\subsubsection{no.uio.ifi.refaktor.prefix}
-This package contains the \type{Prefix} type that is the data representation of
-the prefixes found by the \type{PrefixesCollector}. It also contains the prefix
-set for storing and working with prefixes.
+\subsubsection{Execution time}
+The differences in complexity, and the different candidate methods to total
+number of methods ratios, is shown in the distributions of the execution times.
+For the JDT project, 75\% of the total time was used on the actual changes,
+while for the refaktor project, this number was only 63\%.
-\subsubsection{no.uio.ifi.refaktor.statistics}
-The package contains statistics functionality. Its heart is the statistics
-aspect that is responsible for gathering statistics during the execution of the
-\ExtractAndMoveMethod refactoring.
+For the JDT project, the benchmark used on average 0.21 seconds per method in
+the project, while for the refaktor project it used only 0.07 seconds per
+method. So the process used 3 times as much time per method for the JDT project
+than for the refaktor project.
-\begin{description}
- \item[no.uio.ifi.refaktor.statistics.reports] \hfill \\
- This package contains a simple framework for generating reports from the
- statistics data generated by the aspect. Currently, the only available
- report type is a simple text report.
+While the JDT project is 39 times larger than the refaktor project measured in
+lines of code, the benchmark used about 79 times as long time on it than for the
+refaktor project. Relatively, this is about twice as long.
-\end{description}
+Since the details of these execution times are not that relevant to this
+master's project, only their magnitude, I will leave them here.
+\subsubsection{Executed refactorings}
+For the composite \ExtractAndMoveMethod refactoring performed in case 2, 53
+successful attempts out of 58 gives a success rate of 91.4\%. This is 5.3
+percentage points worse than for the first case.
-\subsubsection{no.uio.ifi.refaktor.textselection}
-This package contains the two custom text selections that are used extensively
-throughout the project. One of them is just a subclass of the other, to support
-the use of the memento pattern to optimize the memory usage during benchmarking.
+\subsection{\name{SonarQube} analysis}
+Results from the \name{SonarQube} analysis are shown in
+\myref{tab:case2ResultsProfile1}.
-\subsubsection{no.uio.ifi.refaktor.debugging}
-The package contains a debug utility class. I addition to this, the package
-\code{no.uio.ifi.refaktor.utils.aspects} contains a couple of aspects used for
-debugging purposes.
+Not much is to be said about these results. The trends in complexity and
+coupling are the same. We end up a little worse after the refactoring process
+than before.
-\subsubsection{no.uio.ifi.refaktor.utils}
-Utility package that contains all the functionality that has to do with parsing
-of source code. It also has utility classes for looking up handles to methods
-and types et cetera.
+\begin{table}[htb]
+ \caption{Results for analyzing the \var{no.uio.ifi.refaktor} project, before
+ and after the refactoring, with \name{SonarQube} and the \name{IFI Refaktor
+ Case Study} quality profile. (Bold numbers are better.)}
+ \label{tab:case2ResultsProfile1}
+ \centering
+ \begin{tabularx}{\textwidth}{@{}>{\bfseries}L{1.5}R{0.25}R{0.25}@{}}
+ \toprule
+ \textnormal{Number of issues for each rule} & Before & After \\
+ \midrule
+ Avoid too complex class & 1 & 1 \\
+ Classes should not be coupled to too many other classes (Single
+ Responsibility Principle) & \textbf{29} & 34 \\
+ Control flow statements \ldots{} should not be nested too deeply & 24 &
+ \textbf{21} \\
+ Methods should not be too complex & 17 & \textbf{15} \\
+ Methods should not have too many lines & 41 & \textbf{40} \\
+ NPath Complexity & 3 & 3 \\
+ Too many methods & \textbf{13} & 15 \\
+ \midrule
+ Total number of issues & \textbf{128} & 129 \\
+ \midrule
+ \midrule
+ \spancols{3}{Complexity} \\
+ \midrule
+ Per function & 2.1 & 2.1 \\
+ Per class & \textbf{12.5} & 12.9 \\
+ Per file & \textbf{13.8} & 14.2 \\
+ \midrule
+ Total complexity & \textbf{2,089} & 2,148 \\
+ \midrule
+ \midrule
+ \spancols{3}{Numbers of each type of entity analyzed} \\
+ \midrule
+ Files & 151 & 151 \\
+ Classes & 167 & 167 \\
+ Functions & 987 & 1,045 \\
+ Accessors & 35 & 30 \\
+ Statements & 3,355 & 3,416 \\
+ Lines of code & 8,238 & 8,460 \\
+ \midrule
+ Technical debt (in days) & \textbf{19.0} & 20.7 \\
+ \bottomrule
+ \end{tabularx}
+\end{table}
-\begin{description}
- \item[no.uio.ifi.refaktor.utils.caching] \hfill \\
- This package contains the caching manager for compilation units, along with
- classes for different caching strategies.
+\subsection{Unit tests}
+The tests used for this case are the same that has been developed throughout
+this master's project.
- \item[no.uio.ifi.refaktor.utils.nullobjects] \hfill \\
- Contains classes for creating different null objects. Most of the classes is
- used to represent null objects of different handle types. These null objects
- are returned from various utility classes instead of returning a \var{null}
- value when other values are not available.
+The code that was refactored for this case suffered from some of the problems
+discovered in the first case. This means that the after-code for this case also
+contained compilation errors, but they were not as many. The code contained only
+6 errors that made the code not compile.
+
+All of the six errors originated from the same bug. The bug arises in a
+situation where a class instance creation is moved between packages, and the
+class for the instance is package-private. The \MoveMethod refactoring does not
+detect that there will be a visibility problem, and neither does it promote the
+package-private class to be public.
+
+Since the errors in the refactored refaktor code were easy to fix manually, I
+corrected them and ran the unit tests as planned. The unit test results are
+shown in \myref{tab:case2UnitTests}. Before the refactoring, all tests passed.
+All tests also passed after the refactoring, with the six error corrections.
+Since the corrections done are not of a kind that could make the behavior of the
+program change, it is likely that the refactorings done to the
+\type{no.uio.ifi.refaktor} project did not change its behavior. This is also
+supported by the informal experiment presented next.
-\end{description}
+\begin{table}[htb]
+ \caption{Results from the unit tests run for the \type{no.uio.ifi.refaktor}
+project, before and after the refactoring (with 6 corrections done to the
+refactored code).}
+ \label{tab:case2UnitTests}
+ \centering
+ \begin{tabularx}{\textwidth}{@{}>{\bfseries}L{1}R{0.5}R{0.5}@{}}
+ \toprule
+ & Before & After \\
+ \midrule
+ Runs & 148/148 & 148/148 \\
+ Errors & 0 & 0 \\
+ Failures & 0 & 0 \\
+ \bottomrule
+ \end{tabularx}
+\end{table}
-\section{Continuous integration}
-The continuous integration server
-\name{Jenkins}\footnote{\url{http://jenkins-ci.org/}} has been set up for the
-project\footnote{A work mostly done by the supervisor.}. It is used as a way to
-run tests and perform code coverage analysis.
+\subsection{An additional experiment}
+To complete the task of ``eating my own dog food'', I conducted an experiment
+where I used the refactored version of the \type{no.uio.ifi.refaktor} project,
+with the corrections, to again refaktor ``itself''.
-To be able to build the \name{Eclipse} plugins and run tests for them with Jenkins, the
-component assembly project
-\name{Buckminster}\footnote{\url{http://www.eclipse.org/buckminster/}} is used,
-through its plugin for Jenkins. Buckminster provides for a way to specify the
-resources needed for building a project and where and how to find them.
-Buckminster also handles the setup of a target environment to run the tests in.
-All this is needed because the code to build depends on an \name{Eclipse}
-installation with various plugins.
+The experiment produced code containing the same six errors as after the
+previous experiment. I also compared the after-code from the two experiments
+with a diff-tool. The only differences found were different method names. This
+is expected, since the method names are randomly generated by the
+\ExtractAndMoveMethod refactoring.
-\subsection{Problems with AspectJ}
-The Buckminster build worked fine until introducing AspectJ into the project.
-When building projects using AspectJ, there are some additional steps that needs
-to be performed. First of all, the aspects themselves must be compiled. Then the
-aspects needs to be woven with the classes they affect. This demands a process
-that does multiple passes over the source code.
+The outcome of this simple experiment makes me more confident that the
+\ExtractAndMoveMethod refactoring made only behavior-preserving changes to the
+\type{no.uio.ifi.refaktor} project, apart from the compilation errors.
-When using AspectJ with \name{Eclipse}, the specialized compilation and the
-weaving can be handled by the \name{AspectJ Development
-Tools}\footnote{\url{https://www.eclipse.org/ajdt/}}. This works all fine, but
-it complicates things when trying to build a project depending on \name{Eclipse}
-plugins outside of \name{Eclipse}. There is supposed to be a way to specify a
-compiler adapter for javac, together with the file extensions for the file types
-it shall operate. The AspectJ compiler adapter is called
-\typewithref{org.aspectj.tools.ant.taskdefs}{Ajc11CompilerAdapter}, and it works
-with files that has the extensions \code{*.java} and \code{*.aj}. I tried to
-setup this in the build properties file for the project containing the aspects,
-but to no avail. The project containing the aspects does not seem to be built at
-all, and the projects that depends on it complains that they cannot find certain
-classes.
+\subsection{Conclusions}
+The differences in complexity between the Eclipse JDT UI project and the
+\type{no.uio.ifi.refaktor} project, clearly influenced the differences in their
+execution times. This is mostly because fewer of the methods were chosen to be
+refactored for the refaktor project than for the JDT project. This makes it
+difficult to know if there are any severe performance penalties associated with
+refactoring on a large project compared to a small one.
-I then managed to write an \name{Ant}\footnote{\url{https://ant.apache.org/}}
-build file that utilizes the AspectJ compiler adapter, for the
-\code{no.uio.ifi.refaktor} plugin. The problem was then that it could no longer
-take advantage of the environment set up by Buckminster. The solution to this
-particular problem was of a ``hacky'' nature. It involves exporting the plugin
-dependencies for the project to an Ant build file, and copy the exported path
-into the existing build script. But then the Ant script needs to know where the
-local \name{Eclipse} installation is located. This is no problem when building
-on a local machine, but to utilize the setup done by Buckminster is a problem
-still unsolved. To get the classpath for the build setup correctly, and here
-comes the most ``hacky'' part of the solution, the Ant script has a target for
-copying the classpath elements into a directory relative to the project
-directory and checking it into Git. When no \code{ECLIPSE\_HOME} property is set
-while running Ant, the script uses the copied plugins instead of the ones
-provided by the \name{Eclipse} installation when building the project. This
-obviously creates some problems with maintaining the list of dependencies in the
-Ant file, as well as remembering to copy the plugins every time the list of
-dependencies change.
+The trends in the \name{SonarQube} analysis are the same for this case as for
+the previous one. This gives more confidence in the these results.
-The Ant script described above is run by Jenkins before the Buckminster setup
-and build. When setup like this, the Buckminster build succeeds for the projects
-not using AspectJ, and the tests are run as normal. This is all good, but it
-feels a little scary, since the reason for Buckminster not working with AspectJ
-is still unknown.
+By refactoring our own code and using it again to refactor our code, we showed
+that it is possible to write an automated composite refactoring that works for
+many cases. That it probably did not alter the behavior of a smaller project
+shows us nothing more than that though, and might just be a coincidence.
-The problems with building with AspectJ on the Jenkins server lasted for a
-while, before they were solved. This is reflected in the ``Test Result Trend''
-and ``Code Coverage Trend'' reported by Jenkins.
+\section{Summary}
+\todoin{Write? Or wrap up in final conclusions?}
+\todoin{``Threats to validity''}
-\chapter{Conclusions and Future Work}
+\chapter{Conclusions and future work}
This chapter will conclude this master's thesis. It will try to give justified
answers to the research questions posed \see{sec:researchQuestions} and present
some future work that could be done to take this project to the next level.
\section{Conclusions}
One of the motivations for this thesis was to create a fully automated composite
refactoring that could be used to make program source code better in terms of
-coupling between classes. It has been shown \todo{show} that a composition of
-the \ExtractMethod and the \MoveMethod refactorings reduces the coupling between
-two classes in an ideal situation. The Eclipse IDE implements both these
-refactorings, as well as providing a framework for analyzing source code, so it
-was considered a suitable tool to build upon for our project.
+coupling between classes. Earlier, in \mysimpleref{sec:CBO}, it was shown that a
+composition of the \ExtractMethod and the \MoveMethod refactorings reduces the
+coupling between two classes in an ideal situation. The Eclipse IDE implements
+both these refactorings, as well as providing a framework for analyzing source
+code, so it was considered a suitable tool to build upon for our project.
The search-based \ExtractAndMoveMethod refactoring was created by utilizing the
analysis and refactoring support of Eclipse, and a small framework was built
SonarQube. The project's own unit tests were also performed to find out if our
refactoring introduces any behavior-altering changes in the code it refactor.
-\paragraph{Answering the main research question}
+\paragraph{Answering the main research question.}
The first and greatest challenge was to find out if the \ExtractAndMoveMethod
refactoring could be automated, in all tasks ranging from analysis to executing
changes. It is now confirmed that this can be done, since it has been
problems.
\paragraph{Is the refactoring efficient enough?}
-By being able to process over 300,000 pure lines of code in about 1.5 hours on a
-mid-level laptop computer, the search-based \ExtractAndMoveMethod refactoring
-must be said to perform reasonably well.
+Since we have concluded that the search-based \ExtractAndMoveMethod refactoring
+is not suitable for on-demand large scale refactoring, but may be put to better
+use as a kind of analysis tool, superb performance is not crucial. By being able
+to process over 300,000 pure lines of code in about 1.5 hours on a mid-level
+laptop computer, the refactoring must be said to perform well enough for this
+purpose. In comparison, the \name{SonarQube} analysis consumes about the same
+amount of time. If performed on demand for a single method, the performance of
+the \ExtractAndMoveMethod refactoring is no issue.
\paragraph{What about breaking the source code?}
The case studies showed that our safety measures that rely on the precondition
\MoveMethod refactorings.
Our refactoring does not take any precautions to preserve behavior. A few
-running \todo{check out test report} and failing unit test for the JDT UI
-project after the refactoring indicate that our refactoring probably causes some
-changes to the way a program behaves.
+running and failing unit test for the JDT UI project after the refactoring
+indicate that our refactoring probably causes some changes to the way a program
+behaves.
\paragraph{Is the quality of the code improved?}
For coupling, there is no evidence that the refactoring improves the quality of
review them and choose whether or not they should be performed.
\section{Future work}
-\todoin{Find out if a complete analysis is feasible}
-\todoin{Complete the analysis}
-\todoin{Make refactorings safer (behavior)}
-\todoin{Improve heuristics/introduce metrics}
+An important part that is missing for making the search-based
+\ExtractAndMoveMethod refactoring more usable, is to complete the
+pre-refactoring analysis of the source code, to make sure that the refactoring
+does not introduce compilation errors when it is performed.
+
+The first point of making the static analysis complete brings up the next
+question: Is it feasible to complete such an analysis? Can this feasibility be
+proven, or disproved?
+
+Another shortcoming of this project is that we have no strategy for assuring
+safety when refactoring, so the code may end up behaving differently after the
+refactoring than it behaved before. Approaches toward safer refactorings are
+mentioned in \myref{sec:saferRefactoring}.
+
+The last important improvement that could be made to this project is to refine
+the heuristics that is used to find suitable refactoring candidates. This effort
+should in particular be directed toward making the heuristics choose candidates
+that do not introduce new dependencies for their destination classes.
\appendix
-\chapter{Eclipse Bugs Submitted}
+\chapter{Eclipse bugs submitted}
\newcommand{\submittedBugReport}[1]{The submitted bug report can be found on
\url{#1}.}
was found when analyzing what kinds of names that were to be considered as
\emph{unfixes} \see{unfixes}.
-\subsection{The bug}
+\paragraph{The bug}
The bug emerges when trying to move a method from one class to another, and when
the target for the move (must be a variable, local or field) is both a parameter
variable and also is assigned to within the method body. \name{Eclipse} allows this to
in Java.
\submittedBugReport{https://bugs.eclipse.org/bugs/show\_bug.cgi?id=420726}
-\subsection{The solution}
+\paragraph{The solution}
The solution to this problem is to add all simple names that are assigned to in
a method body to the set of unfixes.
I discovered
this bug during a batch change on the \type{org.eclipse.jdt.ui} project.
-\subsection{The bug}
+\paragraph{The bug}
This bug surfaces when trying to use the \refa{Move Method} refactoring to move a
method from an anonymous class to another class. This happens both for my
simulation as well as in \name{Eclipse}, through the user interface. It only occurs
-when \name{Eclipse} analyzes the program and finds it necessary to pass an instance of
-the originating class as a parameter to the moved method. I.e. it want to pass a
-\var{this} expression. The execution ends in an
+when \name{Eclipse} analyzes the program and finds it necessary to pass an
+instance of the originating class as a parameter to the moved method. I.e. it
+wants to pass a \var{this} expression. The execution ends in an
\typewithref{java.lang}{IllegalArgumentException} in
\typewithref{org.eclipse.jdt.core.dom}{SimpleName} and its
\method{setIdentifier(String)} method. The simple name is attempted created in
\type{IllegalArgumentException}.
\submittedBugReport{https://bugs.eclipse.org/bugs/show\_bug.cgi?id=429416}
-\subsection{How I solved the problem}
+\paragraph{How I solved the problem}
Since the \type{MoveInstanceMethodProcessor} is instantiated in the
\typewithref{no.uio.ifi.refaktor.change.executors}{MoveMethod\-RefactoringExecutor},
and only need to be a
breaks code}\label{eclipse_bug_429954}
The bug was discovered when doing some changes to the way unfixes is computed.
-\subsection{The bug}
+\paragraph{The bug}
The problem is that \name{Eclipse} is allowing selections that references variables of
local types to be extracted. When this happens the code is broken, since the
extracted method must take a parameter of a local type that is not in the
methods scope. The problem is illustrated in
-\myref{lst:extractMethod_LocalClass}, but there in another setting.
+\myref{lst:extractMethodLocalClass}, but there in another setting.
\submittedBugReport{https://bugs.eclipse.org/bugs/show\_bug.cgi?id=429954}
-\subsection{Actions taken}
+\paragraph{Actions taken}
There are no actions directly springing out of this bug, since the Extract
Method refactoring cannot be meant to be this way. This is handled on the
-analysis stage of our \refa{Extract and Move Method} refactoring. So names representing
-variables of local types is considered unfixes \see{unfixes}.
-\todoin{write more when fixing this in legal statements checker}
+analysis stage of our \refa{Extract and Move Method} refactoring. So names
+representing variables of local types are considered unfixes \see{unfixes}.
+
+
+
\backmatter{}
\printglossaries
\printbibliography
git://git.uio.no / ifi-stolz-refaktor.git / blobdiff