Thursday, 27 September 2018

Mutable members should not be stored or returned directly

public Date getstartDate()
{
return startDate;
}
public void setstartDate(Date startDate)
{
this.startDate = startDate;
}
view raw StartDate1.java hosted with ❤ by GitHub

But SonarLint is complaining about it, so I refactored it into this:

public Date getstartDate()
{
return startDate == null ? null : new Date(startDate.getTime());
}
public void setstartDate(Date startDate)
{
if (startDate != null) {
this.startDate = new Date(startDate.getTime());
} else
{
this.startDate = null;
}
}
view raw StartDate2.java hosted with ❤ by GitHub

It's high time this old codebase gets converted to LocalDates.

References

SonarSource - RSPEC-2384
https://rules.sonarsource.com/java/RSPEC-2384

Monday, 24 September 2018

Using a composite key in a map

Once again I stumbled upon old code, that could use some refactoring.

public class WebSearch implements Component
{
public String getBrand()
{
return "Volkswagen";
}
public String getModel()
{
return "Polo";
}
public Long getTypenumber()
{
return 1L;
}
}
interface Component
{
public String getModel();
public String getBrand();
public Long getTypenumber() ;
}
// brand, model, typenumber, Component
private Map<String, Map<String, Map<Long, Component>>> categoryComponentMap = new HashMap<String, Map<String, Map<Long, Component>>>();
public class ComponentSearcher
{
private final static Long NO_TYPE_NUMBER = Long.MAX_VALUE;
private final static String NO_MODEL = "";
public Component getComponent(
Map<String, Map<String, Map<Long, Component>>> componentMap,
WebSearch search)
{
if (search == null)
{
return null;
}
Long typenumber = search.getTypenumber() != null ? search.getTypenumber() : NO_TYPE_NUMBER;
String model = search.getModel() != null ? search.getModel()
: NO_MODEL;
Map<String, Map<Long, Component>> brandMap = componentMap
.get(search.getBrand());
if (brandMap == null)
{
return null;
}
Map<Long, Component> modelMap = brandMap.get(model);
if (modelMap == null)
{
return null;
}
return modelMap.get(typenumber);
}
}

It took me and my colleague a few seconds to process what the software designer had in mind when he did this.

As it was up to me to add another field to the construction, making it even more terrible than it was, it was time for a refactoring.

Refactored solution

We quite quickly came up with using a Composite Key, which of course is the default way to deal with this sort of thing.

public class CompositeKey
{
public final String model;
public final String brand;
public final Long number;
public CompositeKey(String brand, String model, Long number)
{
this.brand = brand;
this.model = model;
this.number = number;
}
public CompositeKey(Component component)
{
this(component.getBrand(), component.getModel(), component.getTypenumber());
}
public String getBrand()
{
return brand;
}
public String getModel()
{
return model;
}
public Long getTypenumber()
{
return number;
}
public String toString()
{
return brand + "|"+ model + "|"+ number;
}
@Override
public boolean equals(Object o)
{
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
CompositeKey that = (CompositeKey) o;
return Objects.equals(getBrand(), that.getBrand()) &&
Objects.equals(getModel(), that.getModel()) &&
Objects.equals(getTypenumber(), that.getTypenumber());
}
@Override
public int hashCode() {
return Objects.hash(getBrand(), getModel(), getTypenumber());
}
}
interface Component
{
public String getModel();
public String getBrand();
public Long getTypenumber() ;
}
// brand, model, typenumber, Component
public class ComponentSearcher
{
public Component getComponent(
String brand, String model, Long typeNumber,
Map<CompositeKey, Component> componentMap)
{
return componentMap.get(new CompositeKey(brand, model, typeNumber));
}
}

Added benefit

There was some trickery going on to properly deal with NULL keys (which are sometimes not allowed in a Map implementation1).

This disadvantage now disappears with this nice composite key.

Test setup

I tested it with a list of cars, and I do not wish to withhold the test setup:

public class CarCreator
{
// arguments are passed using the text field below this editor
public static void main(String[] args)
{
ComponentSearcher myObject = new ComponentSearcher();
Component volkswagenPolo = new ComponentImpl("Volkswagen", "Polo", 1L);
Component volkswagenGolf = new ComponentImpl("Volkswagen", "Golf", 2L);
Component renaultKwid = new ComponentImpl("Renault", "Kwid", -2L);
Component renaultDuster = new ComponentImpl("Renault", "Duster", -5L);
Component hyundaiGrand = new ComponentImpl("Hyundai", "Grand i10", 12L);
Component datsunGo = new ComponentImpl("Datsun", "GO+", null);
Component mercedesBenz = new ComponentImpl("Mercedes-Benz", "GLE Coupe", 1232L);
Map<CompositeKey, Component> map = new HashMap<>();
map.put(new CompositeKey(volkswagenPolo), volkswagenPolo);
map.put(new CompositeKey(volkswagenGolf), volkswagenGolf);
map.put(new CompositeKey(renaultKwid), renaultKwid);
map.put(new CompositeKey(renaultDuster), renaultDuster);
map.put(new CompositeKey(hyundaiGrand), hyundaiGrand);
map.put(new CompositeKey(datsunGo), datsunGo);
map.put(new CompositeKey(mercedesBenz), mercedesBenz);
Component component = myObject.getComponent("Volkswagen", "Polo", 1L, map);
System.out.println("Found component: " + component);
}
}
view raw CarCreator.java hosted with ❤ by GitHub

Fuzzy matching

Of course it is quite easy to just match on some of the criteria. In this case, we could use a Map containing Lists, where the composite key used has empty fields, and would cause a list of cars to be returned and a full composite key (all search criteria) would just yield a list containing one specific car.

If we wish to use fuzzy matching, we are in a bit of a bind.

I'm pretty sure there are always other (perhaps even better) ways of doing this. If you know of some, let me know.

I'm always interested.

References

[1] Oracle JavaDoc - Map Implementation
https://docs.oracle.com/javase/8/docs/api/java/util/Map.html
Different Types Of Cars List
https://auto.ndtv.com/news/types-of-cars-1450327

Wednesday, 12 September 2018

Pop quiz answer: varargs

The answer for the VarArgs Pop Quiz.

Testcase 1 : [Ljava.lang.String;@579bb367 is not null and has length 0
Testcase 2 : args is null
Testcase 3 : [Ljava.lang.String;@1de0aca6 is not null and has length 2
     first element is null

Process finished with exit code 0

The reason for the quiz is that there's a potential NullPointerException in there, that you need to be aware of.

According to reference [1], if the argument provided is compatible with String[], it is assumed that that is what you meant. And null is always compatible with any T[].

Also interesting enough and just to drive the point home, it means that the method signatures below are identical and Java will give you a big fat warning that you defined the same method twice.

public void vars(String... args);

public void vars(String[] args);

References

[1] Oracle Java Language Spec - 15.12.4.2. Evaluate Arguments
https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.12.4.2
Blogpost - Pop Quiz: Varargs
https://randomthoughtsonjavaprogramming.blogspot.com/2018/09/pop-quiz-varargs.html

Friday, 7 September 2018

Pop quiz : varargs

Pop quiz: what is the output of the following program:

package com.mrbear;
import org.junit.Test;
public class VarArgsTest
{
public void vars(String... args)
{
if (args == null) {
System.out.println("args is null");
return;
}
System.out.println(args + " is not null and has length " + args.length);
if (args.length > 0) {
System.out.println(" first element is " + args[0]);
}
}
@Test
public void test()
{
System.out.print("Testcase 1 : ");vars();
System.out.print("Testcase 2 : ");vars(null);
System.out.print("Testcase 3 : ");vars(null, null);
}
}

The answer in the following blog post.