Linuxn00b Linuxn00b - 1 month ago 7
Java Question

Counting the number of occurrences in an array of "Flower Objects"

I'm attempting to create a method that counts the number of Flower objects (created a class called flower btw) that returns the number of particular flowers in an object flower array.

I'm using a HashMap in order to map an integer (sum amount) to a key (key being the flower object). However, when I output the array, the memory address of the key in the heap alongside the key's location in the HashMap table.

My intention however is to print flower (it's name mainly) with the amount of flowers in that array that are of the same Flower Object type.

My code is as follows:

private void displayFlowers(Flower flowerPack[]) {
// TODO: Display only the unique flowers along with a count of any
// duplicates
/*
* For example it should say Roses - 7 Daffodils - 3 Violets - 5
*/
HashMap<Flower, Integer> flowerFrequency = new HashMap<Flower, Integer>();
for (Flower aFlower : flowerPack) {
if (flowerFrequency.containsKey(aFlower)) {
Integer i = flowerFrequency.get(aFlower);
i++;
} else {
flowerFrequency.put(aFlower, new Integer(1));
}
}
System.out.println(flowerFrequency);
}


The output is like this:

1: Add an item to the pack.
2: Remove an item from the pack.
3: Search for a flower.
4: Display the flowers in the pack.
0: Exit the flower pack interfact.
4
{null=1, Flower@6bc7c054=1, Flower@42a57993=1, Flower@75b84c92=1}


As instructed, I have also added the toString() and equals() methods to the Flower class as follows:

I added the toString() and equals() methods in the Flower class as follows:

public String toString() {
return this.color + " " + this.name + " smells like " + this.scentType + " is Thorny " + this.hasThorns;
}


@Override
public boolean equals(Object otherFlower) {

if (otherFlower == null) {
return false;
}
if (!Flower.class.isAssignableFrom(otherFlower.getClass())) {
return false;
}
final Flower other = (Flower) otherFlower;
if ((this.name == null) ? (other.name != null) : !this.name.equals(other.name)) {
return false;
}
if (!(this.color.equals(other.color))) {
return false;
}

if (!(this.scentType.equals(other.scentType))) {
return false;

}

if (this.hasThorns != other.hasThorns) {
return false;
}

return true;

}

Answer

The problem seems to be how you're interacting with the map. You're both wrapping an int via Integer's constructor (unnecessary due to autoboxing), and you're incrementing a value but never placing it back.

        if (flowerFrequency.containsKey(aFlower)) {
            Integer i = flowerFrequency.get(aFlower); //uses two lookups
            i++; //only increments our immediate value, not the object
        } else {
            flowerFrequency.put(aFlower, new Integer(1)); //does not need wrapping
        }

Ideally, you would retrieve once and place once. HashMap returns null when there is no mapping for the key, so you can use that to your advantage:

Integer amount = flowerFrequency.get(aFlower);
if (amount == null) {
    amount = 0;
}
flowerFrequency.put(aFlower, amount + 1);

Shortened via Java 8:

Integer amount = flowerFrequency.getOrDefault(aFlower, 0); //default 0 for no value
flowerFrequency.put(aFlower, amount + 1);

As for simplifying the problem, Collectors has a nice utility for this:

Map<Flower, Integer> frequency = Arrays.stream(flowerPack)
      .collect(Collectors.groupingBy(Function.identity(), Collectors.summingInt(t -> 1)));

But if you still like utilizing the loop instead, there's also Map#compute for a one-liner solution:

Map<Flower, Integer> frequency = new HashMap<>();
for (Flower f : flowerPack) {
    frequency.compute(f, (key, old) -> old == null ? 1 : old + 1); //increment
}
Comments