3
\$\begingroup\$

I have written a program which closely resembles the way contacts are added and dynamically sorted in ascending order in your phone or other device. Though I have used Integers instead of String, I was just wondering if this has any real world use and how I can improve it.

import java.util.*;

public class Ascending
{

    ArrayList<Integer> array = new ArrayList<Integer>();

    public Ascending()
    {
        array.add(2);
        array.add(4);
        array.add(6);
        array.add(8);
    }

    public static void main()
    {
        Ascending obj = new Ascending();
        obj.ascendingOrder();
    }

    public void ascendingOrder()
    {
        for(int loop=0;loop<=10;loop++)
        {
            Scanner sc = new Scanner(System.in);
            System.out.println("The array is 2,4,6,8 ");
            System.out.println("Enter a number you wish to insert in the array");
            int input = sc.nextInt();
            int i,size = array.size(),last=size-1;

            for(i=0;i<size;i++)
            {
                if(input<array.get(i))
                {
                    array.add(i,input);
                    System.out.println("\nSorted array -:");
                    for(int y:array)
                    {
                        System.out.println(y);

                    }
                    break;
                }
            }

            if(input>array.get(last))
                {
                    array.add(size,input);
                    System.out.println("\nSorted array -:");

                    for(int q:array)
                    {
                        System.out.println(q);
                    }
                }
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ What exactly are you trying to do here? To add an element to a sorted collection? \$\endgroup\$ – h.j.k. Dec 5 '15 at 13:21
  • \$\begingroup\$ Yes that is exactly what I am doing. Just like how it would be in contacts i nyour phone. \$\endgroup\$ – Rayyan Merchant Dec 5 '15 at 14:49
3
\$\begingroup\$
import java.util.*;

It's often preferred to do separate imports rather than import an entire group. Some disagree with this. You can see both viewpoints argued at https://stackoverflow.com/questions/147454/why-is-using-a-wild-card-with-a-java-import-statement-bad

    ArrayList<Integer> array = new ArrayList<Integer>();

It's generally preferred to use the interface on the left side, as so

    List<Integer> numbers = new ArrayList<Integer>();

It won't make much difference in a program like this, but in a program where you pass your field as a method parameter, it can make it much easier to change implementations in the future. It also encourages you to think about whether or not to use implementation specific functionality.

I prefer to use a more specific name than array. Of course, in this case, you don't have much of one as the field is pretty generic. I might also go with data. Some would prefer array to these names.

    public Ascending()
    {
        array.add(2);
        array.add(4);
        array.add(6);
        array.add(8);
    }

Is that test code in your constructor? That's not the place for it. Consider

    public Ascending(List<Integer> initial)
    {
        numbers.addAll(initial);
    }

or better yet, something like

    public Ascending(List<Integer> initialData)
    {
        for (Integer number : initialData)
        {
            numbers.add(number);
        }
    }

As that makes sure that the order is correct (assuming add is implemented correctly).

Then you can change the values in the calling code rather than editing the class itself. I.e. change

        Ascending obj = new Ascending();

to something like

        Ascending ascending = new Ascending(Arrays.asList(2, 4, 6, 8));

Alternately, you could define a new method add and call it in main. This would also allow you to pull some code out of your ascendingOrder method.

    public void ascendingOrder()

Usually methods have verb names. Since this takes input, I might call it something like addFromInput.

Also consider breaking this into three parts: a for loop in main; an input process method; and an add method.

        for(int loop=0;loop<=10;loop++)
        {
            Scanner sc = new Scanner(System.in);

You don't need to make a new Scanner for every iteration. Do it once prior to starting the loop:

        Scanner sc = new Scanner(System.in);
        for (int loop = 0; loop <= 10; loop++)
        {

I also added some whitespace in the for loop declaration. It makes it easier for me to read (the computer won't care).

            System.out.println("The array is 2,4,6,8 ");

Consider writing a toString method so that you can just say

            System.out.println("The array is " + this);

Then it will output the current array on every iteration rather than outputting the original array. This also allows you to change the initial values in only one place.

Alternately, you can write a print method that does the same thing. The toString is more flexible though.

            int i,size = array.size(),last=size-1;

            for(i=0;i<size;i++)
            {
                if(input<array.get(i))
                {
                    array.add(i,input);
                    System.out.println("\nSorted array -:");
                    for(int y:array)
                    {
                        System.out.println(y);

                    }
                    break;
                }
            }

            if(input>array.get(last))
                {
                    array.add(size,input);
                    System.out.println("\nSorted array -:");

                    for(int q:array)
                    {
                        System.out.println(q);
                    }
                }

The indent looks wrong at the end there. However, this whole block of code can be replaced with

            add(input);
            System.out.println("\nSorted array -: " + this);

if you define add and toString as previously discussed.

The add method can be defined as

    public void add(Integer number)
    {
        int i = 0;
        int size = numbers.size();

        while (i < size && number < numbers.get(i))
        {
            i++;
        }

        numbers.add(i, number);
    }

or

    public void add(Integer new)
    {
        int i = 0;

        for (Integer number : numbers)
        {
            if (new < number) {
                numbers.add(i, new);
                return;
            }

            i++;
        }

        numbers.add(new);
    }

among other implementations.

Note that I don't do your last check. It's unnecessary in both versions.

In the first version, i always equals the correct place in the list when it exits the loop.

In the second version, it never reaches the code outside the loop unless it has already checked every previous location. Note that when used with just one argument, add appends the element to the list. This has the same effect as numbers.add(size, new); but is shorter to write.

Moving the printing of the array out of the add process simplifies the logic as well and saves duplicating code.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot but can you tell how to store it permanently and use it as contacts in your phone \$\endgroup\$ – Rayyan Merchant Dec 5 '15 at 8:44
3
\$\begingroup\$

Starters

    Scanner sc = new Scanner(System.in);
    System.out.println("The array is 2,4,6,8 ");

As mentioned in @mdfst13's answer, it's better to create a single instance of Scanner instead of in every iteration, and to print the latest contents of your array List instead of hard-coding to 2,4,6,8 every time.

Creating mutable collections with less code

There's a shorter way to create a mutable List with some preset values:

List<Integer> numbers = new ArrayList<>(Arrays.asList(2, 4, 6, 8));

Arrays.asList(T...) gives us a fixed-size, i.e. immutable List implementation, hence we need to pass that to our own ArrayList object.

Searching in a sorted collection

There's a more efficient method of searching in a sorted collection called binary search, and helpfully there's not one but two appropriately-named Collections.binarySearch() methods that implement this.

Based on your potential usage, you can look at the one that takes in a Comparator, so that you can change the desired ordering.

What's important to note is its return value:

the index of the search key, if it is contained in the list; otherwise, (-(insertion point) - 1). The insertion point is defined as the point at which the key would be inserted into the list: the index of the first element greater than the key, or list.size() if all elements in the list are less than the specified key. Note that this guarantees that the return value will be >= 0 if and only if the key is found. (emphasis mine)

This means it require a bit of special handling when you are adding values not yet present in the List, as the return values are negative. That's something Math.abs(int), i.e. to take the absolute value of an int, can do.

Putting it altogether

private static final Comparator<Integer> COMPARATOR = Comparator.naturalOrder();

public static void main(String[] args) {
    List<Integer> numbers = new ArrayList<>(Arrays.asList(2, 4, 6, 8));
    try (Scanner scanner = new Scanner(System.in)) {
        /* Your choice of looping here */ {
            System.out.println("Before: " + numbers);
            int toAdd = getNumber(scanner);
            int x = Math.abs(Collections.binarySearch(numbers, toAdd, COMPARATOR) + 1);
            numbers.add(x, toAdd);
            System.out.println("After: " + numbers);
        }
    }
}

private static int getNumber(Scanner scanner) {
    System.out.println("Enter a number: ");
    while (!scanner.hasNextInt()) {
        System.out.println("Enter a number: ");
        scanner.next();
    }
    return scanner.nextInt();
}
  • Instead of creating it as a class variable, numbers is used here as a method variable since the problem scope is small enough.
  • try-with-resources is used on the single Scanner instance for safe and efficient handling of the underlying I/O resource.
  • getNumber() returns an int from the Scanner instance, taking care to perform some basic validation by calling scanner.hasNextInt() first.
  • As mentioned above, Math.abs(int) is used to simply convert the new index to a positive number.
  • A before/after output is done to show that numbers remain in sorted order.
\$\endgroup\$
  • \$\begingroup\$ After editing according to your suggestions is there a way that I can use permanently like my address or contact book perhaps.If you could show me a way to do this it would be very helpful \$\endgroup\$ – Rayyan Merchant Dec 5 '15 at 14:52
  • \$\begingroup\$ What's your definition of permanent? \$\endgroup\$ – h.j.k. Dec 5 '15 at 14:55
  • \$\begingroup\$ Something like in your phone like your contacts though not the gui but if i like input some values even after execution they should be stored next time i execute the same program \$\endgroup\$ – Rayyan Merchant Dec 5 '15 at 14:57
  • \$\begingroup\$ You're looking at writing your contents to a file, and then to read it again... I'll suggest doing some experimentation on your own first, ask on StackOverflow to fix any bugs, before posting a new question here about a program that can read and write contents to a file for us to review. :) \$\endgroup\$ – h.j.k. Dec 5 '15 at 14:59

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.