2
\$\begingroup\$

The input is an unsorted array of integers. The output should be an array of integers sorted based on the modulus of 3 (or any other number).

It begins with all integers conforming to Integer % 3 == 0 then 1 and then 2.

public class sortByMod {
public static void main(String args[]) throws Exception {

    int a[] = {1,90,2100, 12, 24, 80, 90, 5};
    LinkedList<List<Integer>> a1=sortMod(a);
    for(int i=0; i <a1.size();i++)
            System.out.print(a1.get(i)+" ");
}

public static LinkedList sortMod(int a[]){
    HashMap<Integer, TreeSet<Integer>> map = new HashMap<Integer, TreeSet<Integer>>();
    for(Integer i =0; i<a.length;i++){
        Integer modKey = a[i]%3;
        if(!map.containsKey(modKey)){
            map.put(modKey, new TreeSet<Integer>());
        }
        TreeSet<Integer> temp = map.get(modKey);
        temp.add(a[i]);
        map.put(modKey,temp);
    }
    LinkedList<List<Integer>> result= new LinkedList<List<Integer>>();
    TreeSet<Integer> temp;
    for(int i=0;i<=2;i++) {

        if(map.containsKey(i)){
           temp = map.get(i);
        }
        else return null;

        List<Integer> list = new ArrayList<Integer>(temp);
        result.add(list);

    }
    return result;
}
}
\$\endgroup\$
0

3 Answers 3

2
\$\begingroup\$

Your code definitely doesn't work with any number except 3, and it's quite likely you'll make an error if you'll be changing the code to another number. So I'd use

public LinkedList sortMod(int a[], int number)

instead. Also, the code returns null when there's no numbers of any modulus. I.e. sortMod(new int[]{1}) returns null as well as sortMod(new int[]{1, 4, 7, 10, 13}).

You can also try to use Streams, feature of Java 8. For example, the single line

return map.values().stream().map(ArrayList::new).collect(Collectors.toList());

does a lot of job, effectively replacing 10 lines of your code. You can try to make the task in two lines using Java 8 features. Also it works fine with sortMod(new int[]{1, 4, 7, 10, 13}).

\$\endgroup\$
2
\$\begingroup\$

Do not use raw-types

Really, don't. The method sortMod is currently returning a raw LinkedList:

public static LinkedList sortMod(int a[])

Consider changing that so that it returns, in your case, a LinkedList<List<Integer>>.

Use the enhanced for loop

The code is currently populating a temporary Map<Integer, TreeSet<Integer> with

for (Integer i = 0; i < a.length; i++) {
    // do something with a[i]
}

When you don't need the index, using an enhanced for loop makes the code more compact and easier to read. Consider having instead

for (Integer element : a) {
    // do something with element
}

Write code against interfaces

The following construct all write code against a specific concrete implementation:

HashMap<Integer, TreeSet<Integer>> map = new HashMap<Integer, TreeSet<Integer>>();
// ...
LinkedList<List<Integer>> result = new LinkedList<List<Integer>>();
// ...
public static LinkedList<List<Integer>> sortMod(int a[])

It is preferable to use top-level interfaces instead, to hide the concrete implementations. Here, it would be:

Map<Integer, TreeSet<Integer>> map = new HashMap<Integer, TreeSet<Integer>>();
// ...
List<List<Integer>> result = new LinkedList<List<Integer>>();
// ...
public static List<List<Integer>> sortMod(int a[])

Also, you don't even need a LinkedList. Except for very specific operations, like adding or removing an element through an iterator, a linked list will be a lot slower than a ArrayList. As such, consider just using a simple ArrayList.

An additional point: since Java 7, you can use the diamond operator and write less concise code with generic classes instantiation. You can just have:

Map<Integer, TreeSet<Integer>> map = new HashMap<>();
// ...
List<List<Integer>> result = new ArrayList<>();

Beware of returning null

When the input array doesn't have an element that is divisible by 3, your code will return null explicitely. This is handled as

for (int i = 0; i <= 2; i++) {
    if (map.containsKey(i)) {
        temp = map.get(i);
    } else
        return null;
    // ...
}

Apart from the missing curly braces, that are always preferable to add, returning null in this case is both surprising and the source of bugs:

  • Surprising, because a caller of the method will not expect

    int array[] = { 1, 3 };
    System.out.println(sortMod(array));
    

    to print null, especially that

    int array[] = { 1, 3, 2 };
    System.out.println(sortMod(array));
    

    would print [[3], [1], [2]].

    A given method should always try to respect the principle of least astonishment. When called with an array of { 1, 3 }, the caller would expect as result [[3], [1], []], that is a list which contains 3 as the only number divisible by 3, then 1 as the only number whose remainder by 3 is 1, and then an empty list since no elements have a remainder of 2 when divided by 3.

  • Source of bugs, because the caller will have to do a null-check before doing any operations, which will often be forgotten, leading to NullPointerExceptions. It is preferable to return empty collections rather than null.

Leverage the Stream API

Starting from Java 8, the code can be greatly simplified using the Stream API (tutorial):

public static List<List<Integer>> sortMod(int a[]) {
    Map<Integer, Set<Integer>> map =
        Arrays.stream(a)
              .boxed()
              .collect(Collectors.groupingBy(i -> i % 3, Collectors.toCollection(TreeSet::new)));
    return map.values().stream().map(ArrayList<Integer>::new).collect(Collectors.toList());
}

This does the same task, but in a handful of lines of code.

  • The first part creates the intermediate map storing the result of the modulo by 3 with their respective numbers. The work is all done by the groupingBy collector, which groups elements classified to a same key and hands those element to a downstream collector. Here, the classifier returns the result of the modulo by 3, and the downstream collector, collecting elements grouped to the same key, collects them into a TreeSet with toCollection.
  • The second part post-processes that map by mapping each value into an ArrayList and collecting all those lists into a new list.
\$\endgroup\$
1
\$\begingroup\$

The quickest way to achive this that I can think of uses the Collections.sort() method and a Comparator.

int a[] = {23,35,7,125,3,12,6,356};
List<Integer> sorted = new LinkedList<>();
for(int i: a){
  sorted.add(i);
}
Collections.sort(sorted, (i1, i2) -> i1%3 > i2%3?1:-1);
System.out.println(sorted);

The Comparator (instanciated by a lmbda expression) compares the mod 3 values of 2 integers and either return that the first is bigget than (return 1) or smaller than (return -1) the second mod 3 value. The sort method will sort the list by that criteria.

You'd have to make a more detailed comparator if you wanted the smallest of the integers to be first if multiple of them have the same mod 3.

My examples output was

[6, 12, 3, 7, 356, 125, 35, 23]
\$\endgroup\$

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.