Skip to content
Snippets Groups Projects
Commit 800e881a authored by Goik Martin's avatar Goik Martin
Browse files

Bugfix in Timeoeriod solution

parent 96470827
No related branches found
No related tags found
No related merge requests found
......@@ -4221,58 +4221,72 @@ public class Circle {
<para>This is a follow-up exercise to <xref
linkend="sd1_qanda_seconds2weeks"/> implementing a corresponding
<classname>Timeperiod</classname> class. Consider the subsequent
example demonstrating both its intended use constructor and
<methodname>toString()</methodname> method:</para>
example:</para>
<informaltable border="0">
<tr>
<td valign="top"><programlisting language="java">final long seconds = 112223;
final Timeperiod t = new Timeperiod(112223);
System.out.println(seconds + " seconds are equal to " + t);</programlisting></td>
final Timeperiod tPeriod = new Timeperiod(112223); <co
linkends="sd1_qanda_seconds2weeks_Class-1"
xml:id="sd1_qanda_seconds2weeks_Class-1-co"/>
System.out.println(seconds + " seconds are equal to " + tPeriod <co
linkends="sd1_qanda_seconds2weeks_Class-2"
xml:id="sd1_qanda_seconds2weeks_Class-2-co"/>); </programlisting><calloutlist>
<callout arearefs="sd1_qanda_seconds2weeks_Class-1-co"
xml:id="sd1_qanda_seconds2weeks_Class-1">
<para>This requires a constructor definition
<methodname>Timeperiod(long)</methodname>.</para>
</callout>
<callout arearefs="sd1_qanda_seconds2weeks_Class-2-co"
xml:id="sd1_qanda_seconds2weeks_Class-2">
<para>This requires a
<methodname>toString()</methodname> method within your
<classname>Timeperiod</classname> class matching
<classname
xlink:href="https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Object.html">Object</classname>.<methodname
xlink:href="https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Object.html#toString()">toString()</methodname>'s
method signature.</para>
</callout>
</calloutlist></td>
<td valign="top"><screen>1 day, 7 hours, 10 minutes and 23 seconds</screen></td>
</tr>
</informaltable>
<para>Since the <methodname>toString()</methodname> method may
not be called at all it shall be implemented on demand in
analogy to <xref linkend="sd1_qanda_defineStringClass"/>. Tip:
create a suitable attribute being <code
language="java">null</code> initially. If
not be called at all it shall be implemented in a »on demand«
fashion similar to <xref
linkend="sd1_qanda_defineStringClass"/>. Hint: create a suitable
attribute being <code language="java">null</code> initially. If
<methodname>toString()</methodname> is being called, first check
for <code language="java">null</code> and initialize it on
demand only.</para>
for <code language="java">null</code> and initialize it by the
desired output if so required.</para>
<para>Furthermore individual weeks, days, hours, minutes and
seconds shall be implemented as read-only values using <code
language="java">final</code>:</para>
seconds shall be implemented as read-only values:</para>
<informaltable border="0">
<tr>
<td valign="top"><programlisting language="java">final Timeperiod t = new Timeperiod(2310983);
System.out.print("weeks = " + t.weeks +
"\ndays = " + t.days +
"\nhours = " + t.hours +
"\nminutes = " + t.minutes +
"\nseconds = " + t.seconds);</programlisting></td>
<td valign="top"><screen>weeks = 3
days = 5
hours = 17
minutes = 56
seconds = 23</screen></td>
<td valign="top"><programlisting language="java">final Timeperiod tPeriod = new Timeperiod(2310983);
System.out.print("weeks = " + tPeriod.weeks);
tPeriod.days = 5; // Expected compile time error:
// Cannot assign a value to final variable 'days'</programlisting></td>
<td valign="top"><screen>weeks = 3</screen></td>
</tr>
</informaltable>
<para>In other words: The <classname>Timeperiod</classname>
class should be implemented as an <link
xlink:href="https://docs.oracle.com/javase/tutorial/essential/concurrency/immutable.html">immutable</link>.
Thus once a <classname>Timeperiod</classname> instance has been
created it shall be impossible to alter the instance's values
<abbrev>i.e.</abbrev> changing its state.</para>
<para>In other words: Instances of
<classname>Timeperiod</classname> should be <link
xlink:href="https://docs.oracle.com/javase/tutorial/essential/concurrency/immutable.html">immutable</link>
objects. Once a <classname>Timeperiod</classname> instance has
been created it shall be impossible to alter its internal
state.</para>
<para>Supply a so called copy constructor to allow for creating
a new instance from an existing one:</para>
<para>In addition supply a so called copy constructor to allow
for creating a new instance from an existing one:</para>
<programlisting language="java">/**
* Clone a given instance.
......@@ -4281,8 +4295,8 @@ seconds = 23</screen></td>
*/
public Timeperiod(final Timeperiod timeperiod) { ... }</programlisting>
<para>Use the following tests to check your
implementation:</para>
<para>Use the following unit tests to check your
implementation's correctness:</para>
<programlisting language="java">public class TimeperiodTest {
......@@ -4318,10 +4332,14 @@ public Timeperiod(final Timeperiod timeperiod) { ... }</programlisting>
expectedSeconds, expectedMinutes, expectedHours, expectedDays, expectedWeeks, expectedToString,
period);
// Also testing copy constructor
// Testing copy constructor
final Timeperiod periodClone = new Timeperiod(period);
Assert.assertTrue("A cloned instance must differ from its original", periodClone != period);
assertPeriodEqualImplement(
expectedSeconds, expectedMinutes, expectedHours, expectedDays, expectedWeeks, expectedToString,
new Timeperiod(period));
periodClone);
}
/**
......@@ -4380,18 +4398,20 @@ public Timeperiod(final Timeperiod timeperiod) { ... }</programlisting>
<answer>
<para>Our implementation basically reads:</para>
<programlisting language="java">public <link
xlink:href="https://gitlab.mi.hdm-stuttgart.de/goik/GoikLectures/blob/master/P/Sd1/Timeperiod/src/main/java/de/hdm_stuttgart/mi/sd1/Timeperiod.java">Timeperiod</link>(final Timeperiod timeperiod) {
<programlisting language="java">public class <link
xlink:href="https://gitlab.mi.hdm-stuttgart.de/goik/GoikLectures/blob/master/P/Sd1/Timeperiod/src/main/java/de/hdm_stuttgart/mi/sd1/Timeperiod.java">Timeperiod</link> {
public final int seconds, minutes, hours, days, weeks;
// Constructors and toString() ...
// Constructors, toString() and other methods
// ...
public final int seconds, minutes, hours, days, weeks; // Instance state
}</programlisting>
<para>The <code language="java">final</code> modifier ensures
instances to be immutable requiring all values to be set within
any constructor. We thus decompose the desired number of seconds
into weeks, days, hours, minutes and remaining seconds:</para>
instances being immutable thus requiring all values to be set
within any constructor being defined. We thus decompose the
desired number of seconds into weeks, days, hours, minutes and
remaining seconds:</para>
<programlisting language="java">public <link
xlink:href="https://gitlab.mi.hdm-stuttgart.de/goik/GoikLectures/blob/master/P/Sd1/Timeperiod/src/main/java/de/hdm_stuttgart/mi/sd1/Timeperiod.java#L65">Timeperiod</link>(int seconds) {
......@@ -4409,11 +4429,85 @@ public Timeperiod(final Timeperiod timeperiod) { ... }</programlisting>
this.seconds = seconds % SECONDS_PER_MINUTE; // remaining seconds
}</programlisting>
<para>Notice the <code language="java">this.seconds</code>
qualification being required to disambiguate the constructor
parameter variable <code language="java">Timeperiod(int
seconds)</code> from the instance member variable <code
language="java">Timeperiod.seconds</code>.</para>
<para>Notice the <code>this.<emphasis
role="red">seconds</emphasis></code> qualification being
required to disambiguate the constructor parameter variable
<code>Timeperiod(int <emphasis
role="red">seconds</emphasis>)</code> scope from the instance
member variable scope <code>Timeperiod.<emphasis
role="red">seconds</emphasis></code>.</para>
<para>The toString() method could be defined
straightforwardly:</para>
<programlisting language="java">public String toString() {
final int largestNonZeroComponent;
if (0 &lt; weeks) {
largestNonZeroComponent = 5; // weeks, days, hours, minutes and seconds
} else if (0 &lt; days) {
largestNonZeroComponent = 4; // days, hours, minutes and seconds
} else if (0 &lt; hours) {
largestNonZeroComponent = 3; // hours, minutes and seconds
} else if (0 &lt; minutes) {
largestNonZeroComponent = 2; // minutes and seconds
} else {
largestNonZeroComponent = 1; // only seconds, potentially zero as well
}
final StringBuffer buffer = new StringBuffer();
switch (largestNonZeroComponent) {
case 5: addSingularOrPlural(buffer, weeks, "week", ", ");
case 4: addSingularOrPlural(buffer, days, "day", ", ");
case 3: addSingularOrPlural(buffer, hours, "hour", ", ");
case 2: addSingularOrPlural(buffer, minutes, "minute", ", ");
case 1: addSingularOrPlural(buffer, seconds, "second", " and ");
}
return buffer.toString();
}</programlisting>
<para>This solution provides the desired result. However if
being called repeatedly it causes a performance penalty. We thus
refine it by a lazy initialization mechanism. In the first step
we simply rename our method having just <code
language="java">private</code> one:</para>
<programlisting language="java">private String toStringImplement() {
final int largestNonZeroComponent;
if (0 &lt; weeks) {
largestNonZeroComponent = 5;
...
return buffer.toString();
}</programlisting>
<para>We now re-implement our desired
<methodname>toString()</methodname> method in a lazy
initialisation fashion by introducing an additional
<code>private</code> attribute:</para>
<programlisting language="java">public class Timeperiod {
// Tedious calculation, will only be initialized on-demand
private String toStringValue = null;
...
public String toString() {
// Calculation is tedious, thus performing it only on-demand.
//
if (null == toStringValue) { // Called for the first time, not yet initialized?
toStringValue = toStringImplement();
}
return toStringValue;
}
}</programlisting>
<para>See <link
xlink:href="https://gitlab.mi.hdm-stuttgart.de/goik/GoikLectures/blob/master/P/Sd1/Timeperiod/src/main/java/de/hdm_stuttgart/mi/sd1/Timeperiod.java">Timeperiod</link>
for a complete solution including a copy constructor
implementation.</para>
</answer>
</qandaentry>
</qandadiv>
......
......@@ -7,33 +7,33 @@ package de.hdm_stuttgart.mi.sd1;
*/
public class Timeperiod {
// Pre-caculated internal values
// Pre-calculated internal values
static private final int
SECONDS_PER_MINUTE = 60, // 60 minutes per hour
SECONDS_PER_HOUR = 60 * SECONDS_PER_MINUTE, // 3600 seconds per hour
SECONDS_PER_DAY = 24 * SECONDS_PER_HOUR, // 86400 seconds per day
SECONDS_PER_WEEK = 7 * SECONDS_PER_DAY; // 604800 seconds per week
// Tedious calculation, will happen only on-demand
// Tedious calculation, will only be initialized on-demand
private String toStringValue = null;
private String toStringImplement() {
final int highestValue;
final int largestNonZeroComponent;
if (0 < weeks) {
highestValue = 5; // weeks, days, hours, minutes and seconds
largestNonZeroComponent = 5; // weeks, days, hours, minutes and seconds
} else if (0 < days) {
highestValue = 4; // days, hours, minutes and seconds
largestNonZeroComponent = 4; // days, hours, minutes and seconds
} else if (0 < hours) {
highestValue = 3; // hours, minutes and seconds
largestNonZeroComponent = 3; // hours, minutes and seconds
} else if (0 < minutes) {
highestValue = 2; // minutes and seconds
largestNonZeroComponent = 2; // minutes and seconds
} else {
highestValue = 1; // only seconds, potentially zero as well
largestNonZeroComponent = 1; // only seconds, potentially zero as well
}
final StringBuffer buffer = new StringBuffer();
switch (highestValue) {
switch (largestNonZeroComponent) {
case 5: addSingularOrPlural(buffer, weeks, "week", ", ");
case 4: addSingularOrPlural(buffer, days, "day", ", ");
case 3: addSingularOrPlural(buffer, hours, "hour", ", ");
......@@ -102,9 +102,9 @@ public class Timeperiod {
*/
@Override
public String toString() {
// Value calculation is tedious, thus doing it only on-demand.
// Calculation is tedious, thus performing it only on-demand.
//
if (null == toStringValue) { // Called for the first time, yet uninitialized?
if (null == toStringValue) { // Called for the first time, not yet initialized?
toStringValue = toStringImplement();
}
return toStringValue;
......
......@@ -40,10 +40,14 @@ public class TimeperiodTest {
expectedSeconds, expectedMinutes, expectedHours, expectedDays, expectedWeeks, expectedToString,
period);
// Also testing copy constructor
// Testing copy constructor
final Timeperiod periodClone = new Timeperiod(period);
Assert.assertTrue("A cloned instance must differ from its original", periodClone != period);
assertPeriodEqualImplement(
expectedSeconds, expectedMinutes, expectedHours, expectedDays, expectedWeeks, expectedToString,
new Timeperiod(period));
periodClone);
}
/**
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment