\usepackage{varioref}
\usepackage[hidelinks]{hyperref}
\usepackage{cleveref}
-\usepackage[xindy,entrycounter]{glossaries}
+\usepackage[xindy]{glossaries}
\usepackage[style=alphabetic,backend=biber]{biblatex}
\usepackage{amsthm}
\newcommand{\myref}[1]{\vref{#1}}
\newcommand{\Myref}[1]{\Vref{#1}}
-\newcommand{\glossref}[1]{\textsuperscript{(\glsrefentry{#1})}}
+%\newcommand{\glossref}[1]{\textsuperscript{(\glsrefentry{#1})}}
%\newcommand{\gloss}[1]{\gls{#1}\glossref{#1}}
%\newcommand{\glosspl}[1]{\glspl{#1}\glossref{#1}}
\newcommand{\gloss}[1]{\gls{#1}}
\newcommand{\MoveMethod}{\refa{Move Method}\xspace}
\newcommand{\ExtractAndMoveMethod}{\refa{Extract and Move Method}\xspace}
+\newcommand{\m}[1]{$#1$}
+
\newcommand\todoin[2][]{\todo[inline, caption={#2}, #1]{
\begin{minipage}{\textwidth-4pt}#2\end{minipage}}}
name=profiler,
description={A profiler is a program for analyzing performance within an
application. It is used to analyze memory consumption, processing time and
-frequency of procedure calls and such.}
+frequency of procedure calls and such}
}
\newglossaryentry{xUnit}
{
identifies its participators and how they collaborate},
plural={design patterns}
}
-\newglossaryentry{extractMethod}
+\newglossaryentry{enclosingClass}
{
- name=\refa{Extract Method},
- description={The \refa{Extract Method} refactoring is used to extract a
-fragment of code from its context and into a new method. A call to the new
-method is inlined where the fragment was before. It is used to break code into
-logical units, with names that explain their purpose}
+ name={enclosing class},
+ description={An enclosing class is the class that surrounds any specific piece
+ of code that is written in the inner scope of this class},
}
-\newglossaryentry{moveMethod}
+\newglossaryentry{mementoPattern}
{
- name=\refa{Move Method},
- description={The \refa{Move Method} refactoring is used to move a method from
- one class to another. This is useful if the method is using more features of
- another class than of the class which it is currently defined. Then all calls
- to this method must be updated, or the method must be copied, with the old
-method delegating to the new method}
+ name={memento pattern},
+ description={The memento pattern is a software design pattern that is used to
+ capture an object's internal state so that it can be restored to this state
+ later\citing{designPatterns}},
}
+%\newglossaryentry{extractMethod}
+%{
+% name=\refa{Extract Method},
+% description={The \refa{Extract Method} refactoring is used to extract a
+%fragment of code from its context and into a new method. A call to the new
+%method is inlined where the fragment was before. It is used to break code into
+%logical units, with names that explain their purpose}
+%}
+%\newglossaryentry{moveMethod}
+%{
+% name=\refa{Move Method},
+% description={The \refa{Move Method} refactoring is used to move a method from
+% one class to another. This is useful if the method is using more features of
+% another class than of the class which it is currently defined. Then all calls
+% to this method must be updated, or the method must be copied, with the old
+%method delegating to the new method}
+%}
\bibliography{bibliography/master-thesis-erlenkr-bibliography}
A problem you often encounter when programming, is that a program contains a lot
of long and hard-to-grasp methods. It can then help to break the methods into
-smaller ones, using the \gloss{extractMethod} refactoring\citing{refactoring}.
-Then you may discover something about a program that you were not aware of
-before; revealing bugs you did not know about or could not find due to the
-complex structure of your program. \todo{Proof?} Making the methods smaller and
-giving good names to the new ones clarifies the algorithms and enhances the
+smaller ones, using the \ExtractMethod refactoring\citing{refactoring}. Then
+you may discover something about a program that you were not aware of before;
+revealing bugs you did not know about or could not find due to the complex
+structure of your program. \todo{Proof?} Making the methods smaller and giving
+good names to the new ones clarifies the algorithms and enhances the
\emph{understandability} of the program \see{magic_number_seven}. This makes
refactoring an excellent method for exploring unknown program code, or code that
you had forgotten that you wrote.
at least they have all passed the first ``refactoring
rubicon''\citing{fowlerRubicon2001,secondRubicon2012}.
-Regarding the \gloss{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}.
+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}.
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
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
-context of test driven development (commonly known as TDD), the tests are even a
+context of test-driven development (commonly known as TDD), the tests are even a
way to define how the program is \emph{supposed} to work. It is then, by
definition, working if the tests are passing.
\chapter{The Project}
-\todoin{Moved from introduction to here. Rewrite and make problem statement from
-it:}
-The aim of this master project will be to investigate the relationship between a
-composite refactoring composed of the \ExtractMethod and \MoveMethod
-refactorings, and its impact on one or more software metrics.
+\section{Project description}
+The aim of this master's project will be to explore the relationship between the
+\ExtractMethod and the \MoveMethod refactorings. This will be done by composing
+the two into a composite refactoring. The refactoring will be called the
+\ExtractAndMoveMethod refactoring.
+
+The two primitive \ExtractMethod and \MoveMethod refactorings must already be
+implemented in a tool, so the \ExtractAndMoveMethod refactoring is going to be
+built on top of those.
The composition of the \ExtractMethod and \MoveMethod refactorings springs
naturally out of the need to move procedures closer to the data they manipulate.
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}},
-but I choose to call it \ExtractAndMoveMethod, since I feel it better
+but I choose to call it \ExtractAndMoveMethod, since I feel this better
communicates which primitive refactorings it is composed of.
-For the metrics, I will at least measure 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 project will then consist in implementing the \ExtractAndMoveMethod
-refactoring, as well as executing it over a larger code base. Then the effect of
-the change must be measured by calculating the chosen software metrics both
-before and after the execution. To be able to execute the refactoring
-automatically I have to make it analyze code to determine the best selections to
-extract into new methods.
-\section{The problem statement}
-\todoin{write/move}
+The project will consist of implementing the \ExtractAndMoveMethod refactoring,
+as well as executing it over a larger code base, as a case study. To be able to
+execute the refactoring automatically, I have to make it analyze code to
+determine the best selections to extract into new methods.
-\section{The refactorings}
+\section{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
+refactoring.
\subsection{The Extract Method refactoring}
The \refa{Extract Method} refactoring is used to extract a fragment of code
\myref{lst:extractMethodRefactoring}. It shows a method containing calls to the
methods \method{foo} and \method{bar} of a type \type{X}. These statements are
then extracted into the new method \method{fooBar}.
-\todoin{Refine}
-\begin{listing}
+\begin{listing}[h]
\begin{multicols}{2}
\begin{minted}[samepage]{java}
// Before
\Myref{lst:moveMethodRefactoring} shows an example of this refactoring. Here a
method \method{fooBar} is moved from the class \type{C} to the class \type{X}.
-\todoin{Refine}
-
-\begin{listing}
+\begin{listing}[h]
\begin{multicols}{2}
\begin{minted}[samepage]{java}
// Before
\label{lst:moveMethodRefactoring}
\end{listing}
-\subsection{The Extract and Move Method refactoring}
-\todoin{Write}
-\missingfigure{Explaining the Extract and Move Method refactoring}
+\section{The Extract and Move Method refactoring}
+The \ExtractAndMoveMethod refactoring is a composite refactoring composed of the
+primitive \ExtractMethod and \MoveMethod refactorings. The effect of this
+refactoring on source code is the same as when extracting a method and moving it
+to another class. Conseptually, this is done without an intermediate step. In
+practice, as we shall see later, an intermediate step may be necessary.
+An example of this composite refactoring is shown in
+\myref{lst:extractAndMoveMethodRefactoring}. The example joins the examples from
+\cref{lst:extractMethodRefactoring} and \cref{lst:moveMethodRefactoring}. This
+means that the selection consisting of the consecutive calls to the methods
+\method{foo} and \method{bar}, is extracted into a new method \method{fooBar}
+located in the class \type{X}.
+
+\begin{listing}[h]
+ \begin{multicols}{2}
+ \begin{minted}[samepage]{java}
+ // Before
+ class C {
+ void method() {
+ X x = new X();
+ x.foo(); x.bar();
+ }
+ }
+
+ class X {
+ void foo(){/*...*/}
+ void bar(){/*...*/}
+ }
+ \end{minted}
+
+ \columnbreak
+
+ \begin{minted}[samepage]{java}
+ // After
+ class C {
+ void method() {
+ X x = new X();
+ x.fooBar();
+ }
+ }
+
+ class X {
+ void fooBar() {
+ foo(); bar();
+ }
+ void foo(){/*...*/}
+ void bar(){/*...*/}
+ }
+ \end{minted}
+ \end{multicols}
+ \caption{An example of the \ExtractAndMoveMethod refactoring.}
+ \label{lst:extractAndMoveMethodRefactoring}
+\end{listing}
+
+\section{Research questions}
+The main question that I seek an answer to in this thesis is:
+
+\begin{quote}
+ Is it possible to automate the analysis and execution of the
+ \ExtractAndMoveMethod refactoring, and do so for all of the code of a larger
+ project?
+\end{quote}
+
+\noindent The secondary questions will then be:
+
+\paragraph{Can we do this efficiently?} Can we automate the analysis and
+execution of the refactoring so it can be run in a reasonable amount of time?
+And what does \emph{reasonable} mean in this context?
+
+And, assuming the refactoring does in fact improve the quality of source code:
+
+\paragraph{How can the automation of the refactoring be helpful?} What is the
+usefullness of the refactoring in a software development setting? In what parts
+of the development process can the refactoring play a role?
\section{Choosing the target language}
Choosing which programming language the code that shall be manipulated shall be
de facto standard IDE for Java development regardless of platform.
-\chapter{Refactorings in Eclipse JDT: Design, Shortcomings and Wishful
-Thinking}\label{ch:jdt_refactorings}
+\chapter{Semantics}
+\todoin{Rename chapter?}
+
+\section{The inputs to the refactoring}
+For executing an \ExtractAndMoveMethod refactoring, there are two simple
+requirements. The first thing the refactoring need is a text selection, telling
+it what to extract. Its second requirement is a target for the subsequent move
+operation.
+
+The extracted method must be called instead of the selection that makes up its
+body. Also, the method call has to be performed via a variable, since the method
+is not static. \todo{Explain why static methods are not considered} Therefore,
+the move target must be a variable in the scope of the extracted selection. The
+actual new location for the extracted method will be the class representing the
+type of the move target variable. But, since the method also must be called
+through a variable, it makes sense to define the move target to be either a
+local variable or a field in the scope of the text selection.
+
+\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
+found for all the candidate selections, so that we are able to sort out the
+selection that is best suited for the refactoring.
+
+To find the best move target for a specific text selection, we first need to
+find all the possible targets. Since the target must be a local variable or a
+field, we are basically looking for names within the selection; names that
+represents references to variables.
+
+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,
+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
+\code{b.x.foo()}. Here, the two references to \code{x}, in the middle of the
+qualified names both preceding \code{foo()}, are not referencing the same
+variable. Even though the variables may share the type, and the method
+\method{foo} thus is the same for both, we would not know through which of the
+variables \var{a} or \var{b} we should call the extracted method.
+
+The possible move targets are then the prefixes that are not among a subset of
+the prefixes that are not valid move targets \see{s:unfixes}. Also, prefixes
+that are just simple names, and have only one occurrence, are left out. This is
+because they are not going to have any positive effect on coupling between
+classes.
+
+For finding the best move target among these safe prefixes, a simple heuristic
+is used. It is as simple as choosing the prefix that is most frequently
+referenced within the selection.
+
+\section{Unfixes}\label{s:unfixes}
+The prefixes that are not valid as move targets are called 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}.
+
+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.
+
+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
+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
+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.)
+
+Neither are local types allowed. This accounts for both local and anonymous
+classes. Anonymous classes are effectively the same as interface types with
+respect to unfixes. Local classes could in theory be used as targets, but this
+is not possible due to limitations of the way the \refa{Extract and Move Method}
+refactoring has to be implemented. The problem is that the refactoring is done
+in two steps, so the intermediate state between the two refactorings would not
+be legal Java code. In the intermediate step for the case where a local class is
+the move target, the extracted method would need to take the local class as a
+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.
+
+\begin{listing}
+\begin{multicols}{2}
+\begin{minted}[]{java}
+// Before
+void declaresLocalClass() {
+ class LocalClass {
+ void foo() {}
+ void bar() {}
+ }
+
+ LocalClass inst =
+ new LocalClass();
+ inst.foo();
+ inst.bar();
+}
+\end{minted}
+
+\columnbreak
+
+\begin{minted}[]{java}
+// After Extract Method
+void declaresLocalClass() {
+ class LocalClass {
+ void foo() {}
+ void bar() {}
+ }
+
+ LocalClass inst =
+ new LocalClass();
+ fooBar(inst);
+}
+
+// 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}
+\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 \texttt{<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 \texttt{this == null}, which would not be meaningful,
+since \var{this} would never be \var{null}.
+
+\todoin{Describe what a text selection is?}
+
+\section{Choosing the selection}
+When choosing a selection between the text selections that have possible move
+targets, the selections need to be ordered. The criteria below are presented in
+the order they are prioritized. If not one selection is favored over the other
+for a concrete criterion, the selections are evaluated by the next criterion.
+
+\begin{enumerate}
+ \item The first criterion that is evaluated is whether a selection contains
+ any unfixes or not. If selection \m{A} contains no unfixes, while
+ selection \m{B} does, selection \m{A} is favored over selection
+ \m{B}. This is done under the assumption that, if possible, avoiding
+ selections containing unfixes will make the code moved a little
+ cleaner.\todoin{more arguments?}
+
+ \item The second criterion that is evaluated is how many possible targets a
+ selection contains. If selection \m{A} has only one possible target, and
+ selection \m{B} has multiple, selection \m{A} is favored. If both
+ selections have multiple possible targets, they are considered equal with
+ respect to this criterion. The rationale for this heuristic is that we would
+ prefer not to introduce new couplings between classes when performing the
+ \ExtractAndMoveMethod refactoring.
+
+ \item When evaluating the last criterion, this is with the knowledge that
+ selection \m{A} and \m{B} both have one possible target. Then, if
+ the move target candidate of selection \m{A} has a higher reference count
+ than the target candidate of selection \m{B}, selection \m{A} is
+ favored. The reason for this is that we would like to move the selection that
+ gets rid of the most references to another class.
+
+\end{enumerate}
+
+If none of the above mentioned criteria favors one selection over another, the
+selections are considered to be equally good candidates for the
+\ExtractAndMoveMethod refactoring.
+
+\section{Disqualifying a selection}
+Certain text selections would lead to broken code if used as input to the
+\ExtractAndMoveMethod refactoring. To avoid this, we have to check all text
+selections for such conditions before they are further analyzed. This sections
+is therefore going to present some properties that makes a selection unsuitable
+for our refactoring.
+
+\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
+would not be safe to move it to another class. The reason for this, is that we
+cannot know if the called method is being overridden by some subclass of the
+\gloss{enclosingClass}, or not.
+
+Imagine that the protected method \method{foo} is declared in class \m{A},
+and overridden in class \m{B}. The method \method{foo} is called from within a
+selection done to a method in \m{A}. We want to extract and move this selection
+to another class. The method \method{foo} is not public, so the \MoveMethod
+refactoring must make it public, making the extracted method able to call it
+from the extracted method's new location. The problem is that the, now public,
+method \method{foo} is overridden in a subclass, where it has a protected
+status. This makes the compiler complain that the subclass \m{B} is trying to
+reduce the visibility of a method declared in its superclass \m{A}. This is not
+allowed in Java, and for good reasons. It would make it possible to make a
+subclass that could not be a substitute for its superclass.
+\todoin{Code example?}
+
+The problem this check helps to avoid, is a little subtle. The problem does not
+arise in the class where the change is done, but in a class derived from it.
+This shows that classes acting as superclasses are especially fragile to
+introducing errors in the context of automated refactoring. This is also shown
+in bug\ldots \todoin{File Eclipse bug report}
+
+\subsection{A double class instance creation}
+The following is a problem caused solely by the underlying \MoveMethod
+refactoring. The problem occurs if two classes are instantiated such that the
+first constructor invocation is an argument to a second, and that the first
+constructor invocation takes an argument that is built up using a field. As an
+example, say that \var{name} is a field of the enclosing class, and we have the
+expression \code{new A(new B(name))}. If this expression is located in a
+selection that is moved to another class, \var{name} will be left untouched,
+instead of being prefixed with a variable of the same type as it is declared in.
+If \var{name} is the destination for the move, it is not replaced by
+\code{this}, or removed if it is a prefix to a member access
+(\code{name.member}), but it is still left by itself.
+
+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.
+\todoin{File Eclipse bug report}
+
+\subsection{Instantiation of non-static inner class}
+When a non-static inner class is instantiated, this must happen in the scope of
+its declaring class. This is because it must have access to the members of the
+declaring class. If the inner class is public, it is possible to instantiate it
+through an instance of its declaring class, but this is not handled by the
+underlying \MoveMethod refactoring.
+
+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
+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
+instances of these enclosing classes of \m{C}.
+
+The problem with this, is that these references may not be valid if they are
+moved to another class. Theoretically, some situations could easily be solved by
+passing, to the moved method, a reference to the instance where the problematic
+referenced member is declared. This should work in the case where this member is
+publicly accessible. This is not done in the underlying \MoveMethod refactoring,
+so it cannot be allowed in the \ExtractAndMoveMethod refactoring either.
+
+\subsection{Inconsistent return statements}
+To verify that a text selection is consistent with respect to return statements,
+we must check that if a selection contains a return statement, then every
+possible execution path within the selection ends in either a return or a throw
+statement. This property is important regarding the \ExtractMethod refactoring.
+If it holds, it means that a method could be extracted from the selection, and a
+call to it could be substituted for the selection. If the method has a non-void
+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
+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.
+
+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
+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-statements and try-statements.
+
+For an if-statement, the rule is that if its then-part does not contain any
+return or throw statements, this is considered illegal. If the then-part does
+contain a return or throw, the else-part is checked. If its else-part is
+non-existent, or it does not contain any return or throw statements, the
+statement is considered illegal. If an if-statement is not considered illegal,
+the bodies of its two parts must be checked.
+
+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.
+
+\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.
+
+This problem occurs in two situations. The first situation arise 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
+only one such assignment, but the selection also contain return statements.
+
+Therefore we must examine the selection for assignments to local variables that
+are referenced after the text selection. Then we must verify that not more than
+one such reference is done, or zero if any return statements are found.
+
+\subsection{Illegal statements}
+An illegal statement may be a statement that is of a type that is never allowed,
+or it may be a statement of a type that is only allowed if certain conditions
+are true.
+
+Any use of the \var{super} keyword is prohibited, since its meaning is altered
+when moving a method to another class.
+
+For a \emph{break} statement, there are two situations to consider: A break
+statement with or without a label. If the break statement has a label, it is
+checked that whole of the labeled statement is inside the selection. If the
+break statement does not have a label attached to it, it is checked that its
+innermost enclosing loop or switch statement also is inside the selection.
+
+The situation for a \emph{continue} statement is the same as for a break
+statement, except that it is not allowed inside switch statements.
+
+Regarding \emph{assignments}, two types of assignments are allowed: Assignments
+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 statements types.}
+
+\subsection{Grand example}
+\todoin{Change title?}
+
+\begin{listing}
+ \begin{minted}[linenos]{java}
+ public class C {
+ public void foo() {
+
+ }
+ }
+
+\end{minted}
+\caption{The grand example}
+\label{lst:grandExample}
+\end{listing}
+
+
+
+
+\chapter{Refactorings in Eclipse JDT: Design and
+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 about
sequence. This problem is not trivial to handle in \name{Eclipse}
\see{hacking_undo_history}.
-\section{Wishful Thinking}
-\todoin{???}
\chapter{Composite Refactorings in Eclipse}
with.
\paragraph{The statement lists creator}
-is responsible for generating lists of statements for all the possible levels of
-statements in the method. The statement lists creator is implemented as an AST
-visitor \see{astVisitor}. It generates lists of statements by visiting all the
-blocks in the method declaration and stores their statements in a collection of
-statement lists. In addition, it visits all of the other statements that can
-have a statement as a child, such as the different control structures and the
-labeled statement.
+is responsible for generating lists of statements for all the possible nesting
+levels of statements in the method. The statement lists creator is implemented
+as an AST visitor \see{astVisitor}. It generates lists of statements by visiting
+all the blocks in the method declaration and stores their statements in a
+collection of statement lists. In addition, it visits all of the other
+statements that can have a statement as a child, such as the different control
+structures and the labeled statement.
The switch statement is the only kind of statement that is not straight forward
to obtain the child statements from. It stores all of its children in a flat
list of statements from a switch statement is therefore traversed, and the
statements between the case statements are grouped as separate lists.
-There is an example of how the statement lists creator would generate lists for
-a simple method in \myref{lst:statementListsExample}.
+\Myref{lst:statementListsExample} shows an example of how the statement lists
+creator would generate lists for a simple method.
\begin{listing}[h]
\def\charwidth{5.7pt}
\end{listing}
\paragraph{The text selections generator} generates text selections for each
-list of statements from the statement lists creator. Conceptually, the generator
-generates a text selection for every possible ordered \todo{make clearer}
-combination of statements in a list. For a list of statements, the boundary
-statements span out a text selection. This means that there are many different
-lists that could span out the same selection.
+list of statements from the statement lists creator. The generator generates a
+text selection for every sub-sequence of statements in a list. For a sequence of
+statements, the first statement and the last statement span out a text
+selection.
In practice, the text selections are calculated by only one traversal of the
statement list. There is a set of generated text selections. For each statement,
represents a text selection.}
\label{lst:textSelectionsExample}
\end{listing}
-\todoin{fix \myref{lst:textSelectionsExample}?}
+\todoin{fix \myref{lst:textSelectionsExample}? Text only? All
+sub-sequences\ldots}
\paragraph{Finding the candidate} for the refactoring is done by analyzing all
the generated text selection with the \type{ExtractAndMoveMethodAnalyzer}
For $n=1$ this is trivial: $\frac{1(1+1)}{2} = \frac{2}{2} = 1$. One statement
equals one selection.
- For $n=2$, you get one text selection for the first statement. For the second,
- you get one selection for the statement itself, and one selection for the two
- of them combined. This equals three selections. $\frac{2(2+1)}{2} =
- \frac{6}{2} = 3$.
+ For $n=2$, you get one text selection for the first statement, one selection
+ for the second statement, and one selection for the two of them combined.
+ This equals three selections. $\frac{2(2+1)}{2} = \frac{6}{2} = 3$.
For $n=3$, you get 3 selections for the two first statements, as in the case
where $n=2$. In addition you get one selection for the third statement itself,
\frac{(k+1)((k+1)+1)}{2} = \sum_{i=1}^{k+1} i$
\end{proof}
+%\definition{A \emph{body of statements} is a sequence of statements where every
+%statement may have sub-statements.}
+\todoin{Define ``body of statements''?}
+
\begin{theorem}
The number of text selections for a body of statements is maximized if all the
statements are at the same level.
\end{theorem}
\begin{proof}
- Assume we have a body of, in total, $k$ statements. Let
- $l,\cdots,m,(k-l-\cdots-m)$ be the lengths of the lists of statements in the
- body, with $l+\cdots+m<k \Rightarrow l,\cdots,m<k$.
+ Assume we have a body of, in total, $k$ statements. Then, the sum of the
+ lengths of all the lists of statements in the body, is also $k$. Let
+ $\{l,\ldots,m,(k-l-\ldots-m)\}$ be the lengths of the lists of statements in
+ the body, with $l+\ldots+m<k \Rightarrow \forall i \in \{l,\ldots,m\} : i < k$.
Then, the number of text selections that are generated for the $k$ statements
is
{
\small
\begin{align*}
- \frac{(k-l-\cdots-m)((k-l-\cdots-m)+ 1)}{2} + \frac{l(l+1)}{2} + \cdots +
- \frac{m(m+1)}{2} = \\
- \frac{k^2 - 2kl - \cdots - 2km + l^2 + \cdots + m^2 + k - l - \cdots - m}{2}
- + \frac{l^2+l}{2} + \cdots + \frac{m^2+m}{2} = \\
- \frac{k^2 + k + 2l^2 - 2kl + \cdots + 2m^2 - 2km}{2}
+ \frac{l(l+1)}{2} + \ldots + \frac{m(m+1)}{2} +
+ \frac{(k-l-\ldots-m)((k-l-\ldots-m)+ 1)}{2} = \\
+ \frac{l^2+l}{2} + \ldots + \frac{m^2+m}{2} + \frac{k^2 - 2kl - \ldots - 2km +
+ l^2 + \ldots + m^2 + k - l - \ldots - m}{2} = \\
+ \frac{2l^2 - 2kl + \ldots + 2m^2 - 2km + k^2 + k}{2}
\end{align*}
}
- It then remains to show that this inequality holds:
+ \noindent It then remains to show that this inequality holds:
\begin{align*}
- \frac{k^2 + k + 2l^2 - 2kl + \cdots + 2m^2 - 2km}{2} < \frac{k(k+1)}{2} =
+ \frac{2l^2 - 2kl + \ldots + 2m^2 - 2km + k^2 + k}{2} < \frac{k(k+1)}{2} =
\frac{k^2 + k}{2}
\end{align*}
- By multiplication by $2$ on both sides, and by removing the equal parts, we get
+ \noindent By multiplication by $2$ on both sides, and by removing the equal
+ parts, we get
\begin{align*}
- 2l^2 - 2kl + \cdots + 2m^2 - 2km < 0
+ 2l^2 - 2kl + \ldots + 2m^2 - 2km < 0
\end{align*}
- Since $l,\cdots,m<k$, we have that $\forall i \in \{l,\cdots,m\} : 2ki > 2i^2$,
- so all the pairs of parts on the form $2i^2-2ki$ are negative. In sum, the
- inequality holds.
+ Since $\forall i \in \{l,\ldots,m\} : i < k$, we have that $\forall i \in
+ \{l,\ldots,m\} : 2ki > 2i^2$, so all the pairs of parts on the form $2i^2-2ki$
+ are negative. In sum, the inequality holds.
\end{proof}
There is a technical challenge with this approach, and it relates to the undo
manager, and the concrete implementation
-UndoManager2\typeref{org.eclipse.ltk.internal.core.refactoring.UndoManager2}.
-This implementation is designed in a way that it is not possible to just add an
-undo change, you have to do it in the context of an active
+\typewithref{org.eclipse.ltk.internal.core.refactoring}{UndoManager2}. This
+implementation is designed in a way that it is not possible to just add an undo
+change, you have to do it in the context of an active
operation\typeref{org.eclipse.core.commands.operations.TriggeredOperations}.
One could imagine that it might be possible to trick the undo manager into
believing that you are doing a real change, by executing a refactoring that is
returning a kind of null change that is returning our composite change of undo
-refactorings when it is performed.
+refactorings when it is performed. But this is not the way to go.
Apart from the technical problems with this solution, there is a functional
problem: If it all had worked out as planned, this would leave the undo history
\subsection{The UnfixesCollector}\label{unfixes}
The \typewithref{no.uio.ifi.refaktor.extractors.collectors}{UnfixesCollector}
-finds unfixes within a selection. That is prefixes that cannot be used as a
-basis for finding a move target in a refactoring.
+finds unfixes within a selection.
+\todoin{Give more technical detail?}
-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}.
-
-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 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 is not
-suitable for moving a methods to. This can be either because it is not
-physically possible to move the 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
-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
-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.) Neither are local types allowed. This accounts for
-both local and anonymous classes. Anonymous classes are effectively the same as
-interface types with respect to unfixes. Local classes could in theory be used
-as targets, but this is not possible due to limitations of the implementation of
-the \refa{Extract and Move Method} refactoring. The problem is that the refactoring is
-done in two steps, so the intermediate state between the two refactorings would
-not be legal Java code. In the case of local classes, the problem is that, in
-the intermediate step, a selection referencing a local class would need to take
-the local class as a parameter if it were to be extracted to a new method. 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.
-
-\begin{listing}
-\begin{multicols}{2}
-\begin{minted}[]{java}
-// Before
-void declaresLocalClass() {
- class LocalClass {
- void foo() {}
- void bar() {}
- }
-
- LocalClass inst =
- new LocalClass();
- inst.foo();
- inst.bar();
-}
-\end{minted}
-
-\columnbreak
-
-\begin{minted}[]{java}
-// After Extract Method
-void declaresLocalClass() {
- class LocalClass {
- void foo() {}
- void bar() {}
- }
-
- LocalClass inst =
- new LocalClass();
- fooBar(inst);
-}
-
-// Intermediate step
-void fooBar(LocalClass inst) {
- inst.foo();
- inst.bar();
-}
-\end{minted}
-\end{multicols}
-\caption{When \refa{Extract and Move Method} tries to use a variable with a local type
-as the move target, an intermediate step is taken that is not allowed. Here:
-\type{LocalClass} is not in the scope of \method{fooBar} in its intermediate
-location.}
-\label{lst:extractMethod_LocalClass}
-\end{listing}
-
-The last class of names that are considered unfixes is names used in null tests.
-These are tests that reads like this: if \texttt{<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 \texttt{this == null}, which would not be meaningful, since
-\var{this} would never be \var{null}.
\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
are described next. They are run in the order presented below.
\subsection{The CallToProtectedOrPackagePrivateMethodChecker}
-This checker is designed to prevent an error that can occur in situations where
-a method is declared in one class, but overridden in another. If a text
-selection contains a call to a method like this, and the seletion is extracted
-to a new method, the subsequent movement of this method could cause the code to
-break.
-
-The code breaks in situations where the method call in the selection is to a
-method that has the \code{protected} modifier, or it does not have any access
-modifiers, i.e. it is package-private. The method is not public, so the
-\MoveMethod refactoring must make it public, making the moved method able to
-call it from its new location. The problem is that the, now public, method is
-overridden in a subclass, where it has a protected or package-private status.
-This makes the compiler complain that the subclass is trying to reduce the
-visibility of a method declared in its superclass. This is not allowed in Java,
-and for good reasons. It would make it possible to make a subclass that could
-not be a substitute for its superclass.
-
-The workings of the \type{CallToProtectedOrPackagePrivateMethod\-Checker} is
-therefore very simple. It looks for calls to methods that are either protected
-or package-private within the selection, and throws an
+This checker is used to check that at selection does not contain a call to a
+method that is protected or package-private. Such a method either has the access
+modifier \code{protected} or it has no access modifier.
+
+The workings of the \type{CallToProtectedOrPackagePrivateMethod\-Checker} is
+very simple. It looks for calls to methods that are either protected or
+package-private within the selection, and throws an
\type{IllegalExpressionFoundException} if one is found.
-The problem this checker helps to avoid, is a little subtle. The problem does
-not arise in the class where the change is done, but in a class derived from it.
-This shows that classes acting as superclasses are especially fragile to
-introducing errors in the context of automated refactoring. This is also shown
-in bug\ldots \todoin{File Eclipse bug report}
+\subsection{The DoubleClassInstanceCreationChecker}
+The \type{DoubleClassInstanceCreationChecker} checks that there are no double
+class instance creations where the inner constructor call take and argument that
+is built up using field references.
+
+The checker visits all nodes of type \type{ClassInstanceCreation} within a
+selection. For all of these nodes, if its parent also is a class instance
+creation, it accepts a visitor that throws a
+\type{IllegalExpressionFoundException} if it enclounters a name that is a field
+reference.
\subsection{The InstantiationOfNonStaticInnerClassChecker}
-When a non-static inner class is instatiated, this must happen in the scope of
-its declaring class. This is because it must have access to the members of the
-declaring class. If the inner class is public, it is possible to instantiate it
-through an instance of its declaring class, but this is not handled by the
-\type{MoveInstanceMethodProcessor} in Eclipse when moving a method. Therefore,
-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,
-the \type{InstantiationOfNonStaticInnerClassChecker} does not validate
-selections that contains instantiations of non-static inner classes. This
-problem is also related to bug\ldots \todoin{File Eclipse bug report}
+The \type{InstantiationOfNonStaticInnerClassChecker} checks that selections
+does not contain instantiations of non-static inner classes. The
+\type{MoveInstanceMethodProcessor} in Eclipse does not handle such
+instantiations gracefully when moving a method. This problem is also related to
+bug\ldots \todoin{File Eclipse bug report}
\subsection{The EnclosingInstanceReferenceChecker}
-The purpose of this checker is to verify that the names in a selection is not
-referencing any enclosing instances. This is for making sure that all references
-is legal in a method that is to be moved. Theoretically, some situations could
-be easily solved my passing a reference to the referenced class with the moved
-method (e.g. when calling public methods), but the dependency on the
+The purpose of this checker is to verify that the names in a text selection are
+not referencing any enclosing instances. In theory, the underlying problem could
+be solved in some situations, but our dependency on the
\type{MoveInstanceMethodProcessor} prevents this.
The
is a modified version of the
\typewithref{org.eclipse.jdt.internal.corext.refactoring.structure.MoveInstanceMethod\-Processor}{EnclosingInstanceReferenceFinder}
from the \type{MoveInstanceMethodProcessor}. Wherever the
-\type{EnclosingInstanceReferenceFinder} would create a fatal error status, the
-checker throws a \type{CheckerException}.
+\type{EnclosingInstanceReferenceFinder} would create a fatal error status, the
+checker will throw a \type{CheckerException}.
-It works by first finding all of the enclosing types of a selection. Thereafter
-it visits all its simple names to check that they are not references to
-variables or methods declared in any of the enclosing types. In addition the
-checker visits \var{this}-expressions to verify that no such expressions is
-qualified with any name.
+The checker works by first finding all of the enclosing types of a selection.
+Thereafter, it visits all the simple names of the selection to check that they
+are not references to variables or methods declared in any of the enclosing
+types. In addition, the checker visits \var{this}-expressions to verify that no
+such expressions are qualified with any name.
\subsection{The ReturnStatementsChecker}\label{returnStatementsChecker}
-The checker for return statements is meant to verify that if a text selection
-contains a return statement, then every possible execution path within the
-selection ends in a return statement. This property is important regarding the
-\ExtractMethod refactoring. If it holds, it means that a method could be
-extracted from the selection, and a call to it could be substituted for the
-selection. If the method has a non-void 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 \type{ExtractMethodRefactoring} of \name{Eclipse}
-appends an empty return statement to the back of the method call. Therefore, the
-analysis does not discriminate on either kinds of return statements, with or
-without a return value.
-
-The property description implies that if the selection is free from return
-statements, then the checker validates. So this is the first thing the checker
-investigates.
+The checker for return statements is meant to verify that a text selection is
+consistent regarding return statements.
+
+If the selection is free from return statements, then the checker validates. So
+this is the first thing the checker investigates.
If the checker proceedes any further, it is because the selection contains one
or more return statements. The next test is therefore to check if the last
-statement of the selection ends in either a return or a throw statement. If the
-last statement of the selection ends in a return statement, then all execution
-paths within the selection should end in either this, or another, return
-statement. This is also true for a throw statement, since it 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 by the \code{return} keyword,
-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
-an if-statement, but the last statement that is executed in the method, and that
-can have any side effects, may be located inside the block of the else part of
-the if-statement.
-
-The responsibility for checking that the last statement of the selection
-eventually ends in a return or throw statement, is put on the
+statement of the selection ends in either a return or a throw statement. The
+responsibility for checking that the last statement of the selection eventually
+ends in a return or throw statement, is put on the
\type{LastStatementOfSelectionEndsInReturnOrThrowChecker}. For every node
-visited, if it is a statement, it does a test to see if the statement is a
+visited, if the node is a statement, it does a test to see if the statement is a
return, a throw or if it is an implicit return statement. If this is the case,
no further checking is done. This checking is done in the \code{preVisit2}
method \see{astVisitor}. If the node is not of a type that is being handled by
-its type specific visit method, the checker performs a simple test. If the node
+its type-specific visit method, the checker performs a simple test. If the node
being visited is not the last statement of its parent that is also enclosed by
the selection, an \type{IllegalStatementFoundException} is thrown. This ensures
that all statements are taken care of, one way or the other. It also ensures
finds the last statement declared in its enclosing method. If this statement is
the same as the one under investigation, it is considered an implicit return
statement. If the statements are not the same, the checker does a search to see
-if statement examined is also the last statement of the method that can be
+if the statement examined is also the last statement of the method that can be
reached. This includes the last statement of a block statement, a labeled
statement, a synchronized statement or a try statement, that in turn is the last
-statement enclosed by the statement types listed. This search goes through all
-the parents of a statement until a statement is found that is not one of the
-mentioned acceptable parent statements. If the search ends in a method
-declaration, then the statement is considered to be the last reachable statement
-of the method, and thus also an implicit return statement.
-
-There are two kinds of statements that are handled explicitly. It is
-if-statements and try-statements. Block, labeled and do-statements are handled
-by fall-through to the other two. Do-statements are considered equal to blocks
-in this context, since their bodies are always evaluated at least one time. If-
-and try-statements are visited only if they are the last node of their parent
-within the selection.
-
-For if-statements, the rule is that if the then-part does not contain any return
-or throw statements, it is considered illegal. If it does contain a return or
+statement enclosed by one of the statement types listed. This search goes
+through all the parents of a statement until a statement is found that is not
+one of the mentioned acceptable parent statements. If the search ends in a
+method declaration, then the statement is considered to be the last reachable
+statement of the method, and thus it is an implicit return statement.
+
+There are two kinds of statements that are handled explicitly: If-statements and
+try-statements. Block, labeled and do-statements are handled by fall-through to
+the other two.
+
+If-statements are handled explicitly by overriding their type-specific visit
+method. If the then-part does not contain any return or throw statements an
+\type{IllegalStatementFoundException} is thrown. If it does contain a return or
throw, its else-part is checked. If the else-part is non-existent, or it does
-not contain any return or throw statements, it is considered illegal. If the
-statement is not regarded illegal, its children are visited.
+not contain any return or throw statements an exception is thrown. If no
+exception is thrown while visiting the if-statement, its children are visited.
-Try-statements are handled much the same way as if-statements. Its body must
+A try-statement is checked very similar to an if-statement. Its body must
contain a return or throw. The same applies to its catch clauses and finally
-body.
+body. Failure to validate produces an \type{IllegalStatementFoundException}.
If the checker does not complain at any point, the selection is considered valid
with respect to return statements.
\subsection{The AmbiguousReturnValueChecker}
-This checker verifies that there are no \emph{ambiguous return statements} in a
-selection. The problem with ambiguous return statements 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. This problem occurs in two situations. The first
-situation arise 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 only one such assignment, but there is also one or
-more return statements in the selection.
-
-First the checker needs to collect some data. Those data are the binding keys
+This checker verifies that there are no ambiguous return values in a selection.
+
+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
\subsection{The IllegalStatementsChecker}
This checker is designed to check for illegal statements.
-Any use of the \var{super} keyword is prohibited, since its meaning is altered
-when moving a method to another class.
-
-For a \emph{break} statement, there is two situations to consider: A break
-statement with or without a label. If the break statement has a label, it is
-checked that whole of the labeled statement is inside the selection. 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 statement, and check that it is contained in the selection. If the break
-statement does not have a label attached to it, it is checked that its innermost
-enclosing loop or switch statement also is inside the selection.
-
-The situation for a \emph{continue} statement is the same as for a break
-statement, except that it is not allowed inside switch 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.
-Regarding \emph{assignments}, two types of assignments is allowed: Assignment to
-a non-final variable and assignment to an array access. All other assignments is
-regarded illegal.
-
-\todoin{Finish\ldots}
+\todoin{Follow the development in the semantics section\ldots}
\chapter{Benchmarking}
\todoin{Better name than ``benchmarking''?}
-This part of the master project is located in the \name{Eclipse} project
+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,
\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 a
-with a fully functional \name{Eclipse} workbench. Most importantly, this gives
-us access to the Java Model of \name{Eclipse} \see{javaModel}.
+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
\section{Optimizations}
When looking for optimizations to make for the benchmarking process, I used the
-\name{VisualVM}\footnote{\url{http://visualvm.java.net/}} for the Java Virtual
-Machine to both profile the application and also to make memory dumps of its
-heap.
+\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 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 was
-run.
+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
application may very well work close to its memory limit for the heap space for
long periods during the benchmark.
-\subsection{Memento}
-\todoin{Write}
+\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.
+
+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.
+
+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.
\chapter{Technicalities}
\section{Source code organization}
-All the parts of this master project is under version control with
+All the parts of this master's project is under version control with
\name{Git}\footnote{\url{http://git-scm.com/}}.
The software written is organized as some \name{Eclipse} plugins. Writing a plugin is
and ``Code Coverage Trend'' reported by Jenkins.
+\chapter{Methodology}
+
+\section{Evolutionary design}
+In the programming work for this project, it have tried to use 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.
+
+The motivation behind evolutionary design is to keep the design of software as
+simple as possible. This means not introducing unneeded functionality into a
+program. You should defer introducing flexibility into your software, until it
+is needed to be able to add functionality in a clean way.
+
+Holding up design decisions, implies that the time will eventually come when
+decisions have to be made. The flexibility of the design then relies on the
+programmer's abilities to perform the necessary refactoring, and \his confidence
+in those abilities. From my experience working on this project, I can say that
+this confidence is greatly enhanced by having automated tests to rely on
+\see{tdd}.
+
+The choice of going for evolutionary design developed naturally. As Fowler
+points out in his article \tit{Is Design Dead?}, evolutionary design much
+resembles the ``code and fix'' development strategy\citing{fowler_design_2004}.
+A strategy that most of us have practiced in school. This was also the case when
+I first started this work. I had to learn the inner workings of Eclipse and its
+refactoring-related plugins. That meant a lot of fumbling around with code I did
+not know, in a trial and error fashion. Eventually I started writing tests for
+my code, and my design began to evolve.
+
+\section{Test-driven development}\label{tdd}
+As mentioned before, the project started out as a classic code and fix
+developmen process. My focus was aimed at getting something to work, rather than
+doing so according to best practice. This resulted in a project that got out of
+its starting blocks, but it was not accompanied by any tests. Hence it was soon
+difficult to make any code changes with the confidence that the program was
+still correct afterwards (assuming it was so before changing it). I always knew
+that I had to introduce some tests at one point, but this experience accelerated
+the process of leading me onto the path of testing.
+
+I then wrote tests for the core functionality of the plugin, and thus gained
+more confidence in the correctness of my code. I could now perform quite drastic
+changes without ``wetting my pants``. After this, nearly all of the semantic
+changes done to the business logic of the project, or the addition of new
+functionality, was made in a test-driven manner. This means that before
+performing any changes, I would define the desired functionality through a set
+of tests. I would then run the tests to check that they were run and that they
+did not pass. Then I would do any code changes necessary to make the tests
+pass. The definition of how the program is supposed to operate is then captured
+by the tests. However, this does not prove the correctness of the analysis
+leading to the test definitions.
+
+\section{Continuous integration}
+\todoin{???}
+
+
\chapter{Eclipse Bugs Found}
\newcommand{\submittedBugReport}[1]{The submitted bug report can be found on
\url{#1}.}
\chapter{Conclusions and Future Work}
\todoin{Write}
+
\section{Future work}
+\todoin{Copied from introduction:}
+For the metrics, I will at least measure 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}.
+
+\ldots
+
+Then the effect of the change must be measured by calculating the chosen
+software metrics both before and after the execution.
\todoin{Metrics, \ldots}
\chapter{Related Work}
git://git.uio.no / ifi-stolz-refaktor.git / blobdiff