1
\$\begingroup\$

The code works as it is written, however; I was wondering if there are any easier or more effective ways to merge two lists.

import java.util.*;
public class MergeTwoSortedLists {
    public static void main(String[] args) 
    {
     java.util.Scanner input = new java.util.Scanner(System.in);

     // Enter values for list1
     System.out.println("Enter the amount of numbers in the first list:");
     // Assign user defined length to the array
     int lengthOfArray = input.nextInt();
     int[] list1 = new int[lengthOfArray];

     // i increments and the user values are assigned to the array until the length is exceeded  
     System.out.println("Enter " + list1.length + " values: ");
     for (int i = 0; i < list1.length; i++)
      list1[i] = input.nextInt();


     // Enter values for list2
     System.out.println("Enter the amount of numbers in the second list:");
     // Assign user defined length to the array
     int lengthOfArray2 = input.nextInt();
     int[] list2 = new int[lengthOfArray2];

     // i increments and the user values are assigned to the array until the length is exceeded
     System.out.println("Enter " + list2.length + " values: ");
     for (int i = 0; i < list2.length; i++)
      list2[i] = input.nextInt();

     // Lists one and two are merged into one list
     int[] list3 = merge(list1, list2);
     // Sorts the lists in numerical order
     Arrays.sort(list3);
     // This accesses the actual user defined integers and not their locations, printing them in an assorted array
     System.out.println("The merged list is: " +Arrays.toString(list3));

    }
    public static int[] merge(int[] list1, int[] list2)
    {
        int[] list3 = new int[list1.length + list2.length];
        // Both p and i increment, and list 3 is assigned the user defined integers from both list 1 and 2
        for (int i = 0, p=0 ; i < list1.length || i < list2.length ; i++) {
            if (i < list1.length) {
                list3[p++] = list1[i];
            }
            if (i < list2.length) {
                list3[p++] = list2[i];
            }
        }
        return list3;

    }

}
\$\endgroup\$
4
  • \$\begingroup\$ Does the code work? How exactly does it work? What does it do? Could you provide some example input and output? Please add more to your question than just the code. \$\endgroup\$ Nov 30, 2015 at 21:49
  • \$\begingroup\$ Do you want to zip two lists in an alternating fashion, or merge two sorted lists such that the result is also sorted? \$\endgroup\$ Nov 30, 2015 at 21:50
  • 1
    \$\begingroup\$ As a note, you'll want to add specifics about what you want people to look at, exactly how this code works, etc. It's technically eligible for reopening now, but still not a good question. \$\endgroup\$
    – anon
    Nov 30, 2015 at 23:04
  • 1
    \$\begingroup\$ I have removed the mergesort tag as this is not about an implementation of that (and Java's Arrays.sort(int[]) is using an implementation of quick sort). \$\endgroup\$
    – h.j.k.
    Dec 1, 2015 at 3:35

3 Answers 3

3
\$\begingroup\$

Formatting

Now let's talk about formatting. Your code is hard to read. Format it. Done.

Actually, if you want some tips:

import java.util.*;
public class MergeTwoSortedLists {
    public static void main(String[] args) 
    {

These lines of code have a couple of problems:

Line 1-2: Space between import statements and class declaration

Line 2-3: Space between class declaration and first line of code inside class

Line 3-4: Java conventions require that braces are on the same line as declaration

Result:

import java.util.*;

public class MergeTwoSortedLists {

    public static void main(String[] args) {

This code:

 java.util.Scanner input = new java.util.Scanner(System.in);

 // Enter values for list1
 System.out.println("Enter the amount of numbers in the first list:");
 // Assign user defined length to the array
 int lengthOfArray = input.nextInt();
 int[] list1 = new int[lengthOfArray];

 // i increments and the user values are assigned to the array until the length is exceeded  
 System.out.println("Enter " + list1.length + " values: ");
 for (int i = 0; i < list1.length; i++) // line 11
  list1[i] = input.nextInt();


 // Enter values for list2
 System.out.println("Enter the amount of numbers in the second list:");
 // Assign user defined length to the array
 int lengthOfArray2 = input.nextInt();
 int[] list2 = new int[lengthOfArray2];

 // i increments and the user values are assigned to the array until the length is exceeded
 System.out.println("Enter " + list2.length + " values: ");
 for (int i = 0; i < list2.length; i++) // line 23
  list2[i] = input.nextInt();

 // Lists one and two are merged into one list
 int[] list3 = merge(list1, list2);
 // Sorts the lists in numerical order
 Arrays.sort(list3);
 // This accesses the actual user defined integers and not their locations, printing them in an assorted array
 System.out.println("The merged list is: " +Arrays.toString(list3));

all share a problem. That problem is that you are indenting it by only 1 space, Java conventions recommend 4-space tabs, or just 4 spaces.

Some other things:

Line 11 (as marked by comment): ALWAYS put braces to group if, else, while, do-while, for, and similar statements. Here is why.

Line 13 or 14: Two empty lines won't improve readability. Remove one.

Line 23 (as marked by a comment): Same thing as Line 11

Last line: Space before and after +

Result:

    java.util.Scanner input = new java.util.Scanner(System.in);

    // Enter values for list1
    System.out.println("Enter the amount of numbers in the first list:");
    // Assign user defined length to the array
    int lengthOfArray = input.nextInt();
    int[] list1 = new int[lengthOfArray];

    // i increments and the user values are assigned to the array until the length is exceeded  
    System.out.println("Enter " + list1.length + " values: ");
    for (int i = 0; i < list1.length; i++) {
        list1[i] = input.nextInt();
    }

    // Enter values for list2
    System.out.println("Enter the amount of numbers in the second list:");
    // Assign user defined length to the array
    int lengthOfArray2 = input.nextInt();
    int[] list2 = new int[lengthOfArray2];

    // i increments and the user values are assigned to the array until the length is exceeded
    System.out.println("Enter " + list2.length + " values: ");
    for (int i = 0; i < list2.length; i++) {
        list2[i] = input.nextInt();
    }

    // Lists one and two are merged into one list
    int[] list3 = merge(list1, list2);
    // Sorts the lists in numerical order
    Arrays.sort(list3);
    // This accesses the actual user defined integers and not their locations, printing them in an assorted array
    System.out.println("The merged list is: " + Arrays.toString(list3));

Here:

public static int[] merge(int[] list1, int[] list2)
{
    int[] list3 = new int[list1.length + list2.length];
    // Both p and i increment, and list 3 is assigned the user defined integers from both list 1 and 2
    for (int i = 0, p=0 ; i < list1.length || i < list2.length ; i++) {
        if (i < list1.length) {
            list3[p++] = list1[i];
        }
        if (i < list2.length) {
            list3[p++] = list2[i];
        }
    }
    return list3;
    // line 15
}

Looks pretty good, but some points:

Line 1: Space between methods

Line 1-2: Java conventions require that braces are on the same line as declaration

Line 4: Space before and after =
Line 4: Usually no space is required before ;

Line 15: Extra newline at the end of the method is not necessary

Result:

public static int[] merge(int[] list1, int[] list2) {
    int[] list3 = new int[list1.length + list2.length];
    // Both p and i increment, and list 3 is assigned the user defined integers from both list 1 and 2
    for (int i = 0, p = 0; i < list1.length || i < list2.length; i++) {
        if (i < list1.length) {
            list3[p++] = list1[i];
        }
        if (i < list2.length) {
            list3[p++] = list2[i];
        }
    }
    return list3;
}

Review

import java.util.*;

Never import an entire package, even if you are using a lot of the package. In this case, all you need is `Scanner, so that's even worse.

import java.util.Scanner;

is what you should do.

java.util.Scanner input = new java.util.Scanner(System.in);

Why are you using the full name when you just imported it?

Scanner input = new Scanner(System.in);

Now, to your merge method:

  • The method should be private
  • Since you are sorting the array anyways, it is more efficient to sort the arrays, then merge them, like the final step of merge sort:

    private static int[] merge(int[] array1, int[] array2) {
        int length1 = array1.length;
        int length2 = array2.length;
    
        int[] array = new int[length1 + length2];
    
        int index1 = 0;
        int index2 = 0;
    
        while (index1 < length1 && index2 < length2) {
            array[index1 + index2] = array1[index1] < array2[index2]
                    ? array1[index1++]
                    : array2[index2++];
        }
        while (index1 < length1) {
            array[index1 + index2] = array1[index1++];
        }
        while (index2 < length2) {
            array[index1 + index2] = array2[index2++];
        }
        return array;
    }
    

Final Code:

import java.util.Scanner;

public class MergeTwoSortedLists {

    public static void main(String[] args) {
        Scanner input = new Scanner(System.in);

        // Enter values for list1
        System.out.println("Enter the amount of numbers in the first list:");
        // Assign user defined length to the array
        int lengthOfArray = input.nextInt();
        int[] list1 = new int[lengthOfArray];

        // i increments and the user values are assigned to the array until the length is exceeded  
        System.out.println("Enter " + list1.length + " values: ");
        for (int i = 0; i < list1.length; i++) {
            list1[i] = input.nextInt();
        }

        // Enter values for list2
        System.out.println("Enter the amount of numbers in the second list:");
        // Assign user defined length to the array
        int lengthOfArray2 = input.nextInt();
        int[] list2 = new int[lengthOfArray2];

        // i increments and the user values are assigned to the array until the length is exceeded
        System.out.println("Enter " + list2.length + " values: ");
        for (int i = 0; i < list2.length; i++) {
            list2[i] = input.nextInt();
        }
        // Sorts the lists in numerical order
        Arrays.sort(list1);
        Arrays.sort(list2);

        // Lists one and two are merged into one list
        int[] list3 = merge(list1, list2);
        // This accesses the actual user defined integers and not their locations, printing them in an assorted array
        System.out.println("The merged list is: " + Arrays.toString(list3));
    }

    private static int[] merge(int[] array1, int[] array2) {
        int length1 = array1.length;
        int length2 = array2.length;

        int[] array = new int[length1 + length2];

        int index1 = 0;
        int index2 = 0;

        while (index1 < length1 && index2 < length2) {
            array[index1 + index2] = array1[index1] < array2[index2]
                    ? array1[index1++]
                    : array2[index2++];
        }
        while (index1 < length1) {
            array[index1 + index2] = array1[index1++];
        }
        while (index2 < length2) {
            array[index1 + index2] = array2[index2++];
        }
        return array;
    }

}
\$\endgroup\$
2
  • \$\begingroup\$ Is this the link you were looking for? The answer contains a link to a big apple bug that happened because of this. \$\endgroup\$
    – SirPython
    Dec 1, 2015 at 0:19
  • \$\begingroup\$ @SirPython Not exactly, but this works. Thanks! \$\endgroup\$ Dec 1, 2015 at 0:22
2
\$\begingroup\$

In your code, you are repeating logic for reading a list of ints. That means that this can be extracted into a separate method:

public static int[] readList(Scanner input) {
    System.out.println("Enter the length of the list:");
    int length = input.nextInt();

    System.out.println("Enter " + length + " values:");

    int[] list = new int[length];
    for(int i = 0; i < length; i++) {
        list[i] = input.nextInt();
    }

    return list;
}

Note the parameter input. By having this here, you can create unit tests for this method and you can pass in predefined input without having to read from stdin as the main method is doing.

Now, instead of repeating this logic in your main method, you can just use this new method:

int[] list1 = readList(input);
int[] list2 = readList(input);
... merging stuff ...

 // Lists one and two are merged into one list
 int[] list3 = merge(list1, list2);
 // Sorts the lists in numerical order
 Arrays.sort(list3);

Wait, what? You're going through all that hassle of merging the arrays, only to have them sorted afterwards?

If you don't really care how you are merging the arrays, then you can merge them in a much simpler way using System.arraycopy:

int[] finalList = new int[list1.length + list2.length];
System.arraycopy(list1, 0, final, 0, list1.length);
System.arraycopy(list2, 0, final, list1.length, list2.length);
\$\endgroup\$
0
1
\$\begingroup\$

If you happen to be on Java 8, using IntStream is an alternative to using System.arrayCopy() as suggested in @SirPython's answer:

int[] result = IntStream.concat(IntStream.of(list1), IntStream.of(list2))
                        .sorted().toArray();

A more general solution to combining \$n\$ arrays is to use Arrays.stream() before reducing them into a single IntStream:

private static IntStream combine(int[]... arrays) {
    return Arrays.stream(arrays).flatMapToInt(IntStream::of);
}

// Usage
int[] result = combine(list1, list2, /* ... */ listN).sorted().toArray();
  • Create a Stream<int[]> from the arrays.
  • flatMapToInt() by converting each int[] into an IntStream and combining them together to give the resulting IntStream.
\$\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.