3
\$\begingroup\$

Code shows duplicate values of integers in an array with indexes and the number of occurrences. Can you please critique my code and provide your thoughts on where I should improve my code?

import java.util.Set;
import java.util.HashMap;
import java.util.Map;
import java.util.HashSet;

public class DuplicatesInAnArray {

    private static class ShowIndexDuplicateData<T> {
        private static final String NEW_LINE = System
                .getProperty("line.separator");
        private Set<T> indices;
        private int count;

        public ShowIndexDuplicateData(Set<T> indices, int count) {
            this.indices = indices;
            this.count = count;
        }

        public Set<T> getIndices() {
            return indices;
        }

        public int getCount() {
            return count;
        }

        @Override
        public String toString() {
            StringBuilder sb = new StringBuilder();
            sb.append("Count: ").append(getCount()).append(" Indices: ")
                    .append(getIndices()).append(NEW_LINE);

            return sb.toString();
        }
    }

    public static void duplicatesInAnArray(Integer[] array) {
        if (array == null || array.length <= 1) {
            System.out.println("");
        }

        Map<Integer, ShowIndexDuplicateData<Integer>> map = new HashMap<>();

        for (int i = 0; i < array.length - 1; i++) {

            if (map.containsKey(array[i])) {
                ShowIndexDuplicateData<Integer> showIndexDuplicateData = map
                        .get(array[i]);
                showIndexDuplicateData.count++;
                showIndexDuplicateData.indices.add(i);
            } else {
                int count = 1;
                Set<Integer> indices = new HashSet<>();
                indices.add(i);
                ShowIndexDuplicateData<Integer> showIndexDuplicateData = new ShowIndexDuplicateData<>(
                        indices, count);
                map.put(array[i], showIndexDuplicateData);
            }
        }
        System.out.println(map);
    }

    public static void main(String[] args) {
        Integer[] array = { 5, 6, 1, 2, 3, 3, 6, 1 };
        DuplicatesInAnArray.duplicatesInAnArray(array);
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ Are you on Java 8? \$\endgroup\$
    – h.j.k.
    Commented Aug 30, 2015 at 6:46
  • \$\begingroup\$ @h.j.k. : yes i am. \$\endgroup\$
    – crondx
    Commented Aug 30, 2015 at 14:15
  • \$\begingroup\$ Wells you got a pretty great Java 8 answer there... :) \$\endgroup\$
    – h.j.k.
    Commented Aug 30, 2015 at 14:18

1 Answer 1

8
\$\begingroup\$
  1. You don't need to use Set since indexes can never repeat. Use a List isntead.
  2. You don't need to keep track of the count. You can just get indices.size(). That makes ShowIndexDuplicateData class unnecessary.
  3. You probably should be dealing with int[] rather than Integer[]. There aren't many reasons to have a Integer[]

With that, you can really simplify this (using java 8):

import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toList;
.....


static final String FORMAT = "%s: count=%s, indices=%s%n";  

public static void duplicatesInArray(int[] ary) {
    IntStream.range(0,ary.length).boxed()
            .collect(groupingBy(i -> ary[i], toList()))
            .forEach((num, lst) -> System.out.printf(FORMAT, num, lst.size(), lst));
}
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.