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\$
  • \$\begingroup\$ Are you on Java 8? \$\endgroup\$ – h.j.k. Aug 30 '15 at 6:46
  • \$\begingroup\$ @h.j.k. : yes i am. \$\endgroup\$ – crondx Aug 30 '15 at 14:15
  • \$\begingroup\$ Wells you got a pretty great Java 8 answer there... :) \$\endgroup\$ – h.j.k. Aug 30 '15 at 14:18
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\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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