diff --git a/report.pdf b/report.pdf index dcf49e1..899bee2 100644 Binary files a/report.pdf and b/report.pdf differ diff --git a/report.tex b/report.tex index 0ef67eb..350dedc 100644 --- a/report.tex +++ b/report.tex @@ -168,29 +168,27 @@ the library specifies Java 1.6 as a build target, which is not supported by JDK 17 or above. We used JDK 11 instead, as it is the most recent LTS version of the JDK to still support this target. -An XML dump of the \textit{Pattern4j} analysis results is included in the +An XML dump of the \textit{Pattern4J} analysis results is included in the submission as the file \textit{analysis.xml}. In the following sections each detection of the \textit{Pattern4J} tool is reviewed to characterize if it is indeed not a false positive and if the design pattern is varied in any way in its application. For the sake of brevity, -when referring to a class by its fully-qualified nomain name the prefix +when referring to a class by its fully-qualified domain name the prefix \textit{com.fasterxml.jackson.core} is omitted as all classes in the Jackson core project reside in this package or in a sub-package of this package. \section{Structural Patterns} -\subsection{TO REWRITE Singleton Pattern} +\subsection{Singleton Pattern} -Ensure a class only has one instance and provide a global point of access to it. -The tool found thirteen instances with the Singleton pattern. Doing a deeper -analysis of the found instances, we discovered that some results are false -positives. Example, \textit{sym.Name1} has a private -package constructor and a public static final instance of it, but reading the -documentation, the class represents (short) JSON string literals and is -initialized by client code. +\textit{Pattern4J} found a lot of instances of the singleton pattern, namely 13. +However, some discussion is required to understand the ways the Jackson core +project applies this pattern, as the instances found are sometimes wildly +different from the standard application or outright false positives. -\begin{lstlisting}[caption=Name1 class,language=java] +\begin{lstlisting}[caption=The \textit{sym.Name1} +class.,language=java,label=lst:name1] public final class Name1 extends Name { private final static Name1 EMPTY = new Name1("", 0, 0); private final int q; @@ -226,17 +224,44 @@ public final class Name1 extends Name { } \end{lstlisting} +For example, \textit{sym.Name1} (whose sources are in Listing \ref{lst:name1}) +has a package-private constructor and a \textit{public static final} instance of +itself. This is enough for \textit{Pattern4J} to flag the class as a singleton, +as its constructor is never called in Jackson core other than for initializing +the aforementioned field. However, by reading the documentation it is clear +that the class is meant to be instantiated multiple times. Indeed, its purpose +is to box and represent JSON string literals shorter than 4 bytes, implying the +class is meant to be initialized by clients of the core Jackson module. + +Several less-than-obvious results like this one are reported by the tool, +namely: + \begin{description} \item[sym.Name1, JsonLocation, DefaultIndenter, - util.DefaultPrettyPrinter\$FixedSpaceIndenter] is not a singleton (detected cause of "convenient" default instance given as static final field), the constructor is not used, but the class is extensible - \item[JsonPointer, filter.TokenFilter] is like the above, but constructors are protected + util.DefaultPrettyPrinter\$FixedSpaceIndenter] are not singletons and this + false positives. All these classes were detected because of ``default'' + instances they include in themselves as \textit{static final} fields and + because their constructor, even if \textit{public}, is never used in Jackson + core itself. However, by checking the documentation all these classes are + meant to be extended and instantiated in other Jackson modules; + \item[JsonPointer, filter.TokenFilter] are as described above, however having + \textit{protected} constructors; \item[JsonpCharacterEscapes, util.DefaultPrettyPrinter\$NopIndenter, - Version] a singleton but with a public constructor that is never called in the module code, - may be called in tests - \item[io.JsonStringEncoder] like above, but the class is final - \item[util.InternCache, io.CharTypes\$AltEscapes] - actual singleton, thread-unsafe initialization - \item[io.ContentReference] like above, but constructor is protected + Version] may be considered variations of the singleton pattern that however + include a \textit{public} constructor that is never called in the module + code, but that may be called in tests. Given the \textit{public} + constructors, these classes are hardly solid singleton implementations. + However, we gave the benefit of the doubt to Jackson developers as the use + of the constructors in test code may hint to a purposefully open design to + allow for testability; + \item[io.JsonStringEncoder] is as described above, however the class is + declared as \textit{final}; + \item[util.InternCache, io.CharTypes\$AltEscapes] are both rather standard + singleton pattern applications, however implemented with eager (non-lazy) + initialization (i.e.\ storing the instance in a \textit{public static final} + field); + \item[io.ContentReference] is as described above, however having a + \textit{protected} constructor instead of a \textit{private} one. \end{description} \subsection{Abstract Factory Pattern} @@ -273,7 +298,7 @@ A manual search in the source code produced the following results: TBD -\subsection{Decorator Pattern} +\subsection{TBD Decorator Pattern} Decorator pattern lets you dynamically change the behavior of an object at run time by wrapping them in an object of a decorator class. (com.fasterxml.jackson.core omitted for brevity) @@ -283,7 +308,7 @@ Decorator pattern lets you dynamically change the behavior of an object at run t \item[JsonParser] TBD \end{description} -Only in Pattern4j +Only in Pattern4J \subsection{Bridge Pattern} TBD @@ -341,7 +366,7 @@ pattern); \end{description} \subsection{Strategy Pattern} -\textit{Pattern4j} detects no instance of the strategy pattern in Jackson, +\textit{Pattern4J} detects no instance of the strategy pattern in Jackson, however the previous section regarding the state pattern referenced some false positives that were indeed applications of this pattern. Due to the flexibility of Jackson, there are many more instances of the strategy pattern to configure