Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'd like this code to be improved.

package com.array.demo;

import java.util.Arrays;

public class ArraysDemo {

public static void main(String[] args) {
    // TODO Auto-generated method stub

    // array initialization
    int[] arr = { 5, 10, 12, 22, 4 };

    int size = arr.length;

    int i,pos; // loop counter

    System.out.println(arr); // // prints something like '[I@3343c8b3'

    System.out.println("Elements in the array" + Arrays.toString(arr));

    System.out.println(" The size of the array is  " + size);

    // array-search
    int searchKey = 11;

    for ( i = 0; i < size; i++) {
        if (arr[i] == searchKey)
            break;
    }

    if(i==size)
        System.out.println(" Element not found");
    else
        System.out.println("Element found in the position " +i);


    int elemDel=5;


    for ( pos = 0; pos < size; pos++) {
        if (arr[pos] == elemDel)
            break;
    }

    if(pos==size)
        System.out.println("Element not found");
    else
    {
        while(pos<size-1){
            arr[pos] = arr[pos+1];
            pos++;
        }
        size--;
    }

// display the element after deletion
for(i=0;i<size;i++){
    System.out.print(arr[i] + " ");
}
    //System.out.println("Elements in the array" + Arrays.toString(arr));




}

}
share|improve this question
    
Your previous program used separate methods, but not this one. Is there a reason for that? –  Jamal Jul 19 at 16:01
    
Jamal, I just want to make this program simple and clear. Just i wrote this for understanding the logic. –  Arun Prakash Jul 19 at 16:16
4  
I understand. I was just curious since having separate methods does accomplish this. It does not at all hinder the logic. –  Jamal Jul 19 at 16:18

1 Answer 1

up vote 5 down vote accepted

The easy stuff first...

You should format your code nicer. It makes it easier to read for everyone who uses the same standard formatting style. Use your IDE's reformatting function (Control + Shift + f in Eclipse, Alt + Control + l in IntelliJ) to reformat before posting on forums.

Remove pointless comments. The auto-generated TODO has no place in working code. And this one for example is really stating the obvious:

// array initialization
int[] arr = { 5, 10, 12, 22, 4 };

In general, try to not write comments. If your code is not self-explanatory without comments, try to rewrite it. A common technique is to extract a few lines of code into a method, and the name of the method will explain what those lines do instead of a comment.

You probably you know you're reinventing the wheel, but just for the record, there is an ArrayList<T> class that can do all the things you're doing in your code.

Object-oriented design

The code in the main method is doing all kinds of things, all of them centered around an array, and various manipulations on it, such as:

  • find the index of an element
  • remove an element, if exists
  • print elements

It would make sense to encapsulate all these by a class to keep track of the array and its size. Maybe something like this:

public class ArraysDemo {

    private final int[] values;
    private int size;

    public ArraysDemo(int... values) {
        this.values = values.clone();
        this.size = values.length;
    }

    int indexOf(int value) {
        for (int i = 0; i < size; i++) {
            if (values[i] == value) {
                return i;
            }
        }
        return -1;
    }

    boolean remove(int value) {
        for (int i = 0; i < size; i++) {
            if (values[i] == value) {
                --size;
                for (; i < size; ++i) {
                    values[i] = values[i + 1];
                }
                return true;
            }
        }
        return false;
    }

    void print() {
        System.out.print("Elements in the array: ");
        for (int i = 0; i < size; ++i) {
            System.out.print(values[i] + " ");
        }
        System.out.println();
    }
}

I tried to mimic the methods in ArrayList<T>. This is a good idea to do when you're reinventing the wheel. For example indexOf to find the index of an element, or return -1 if not found. A remove method that returns true if something was removed.

Using this class, you can rewrite the main method like this:

ArraysDemo arr = new ArraysDemo(5, 10, 12, 22, 4);
arr.print();

int index = arr.indexOf(11);
if (index < 0) {
    System.out.println("Element not found");
} else {
    System.out.println("Element found in the position " + index);
}

if (!arr.remove(5)) {
    System.out.println("Element not found");
}
arr.print();

As a final step, the class can be generalized to work with any type, not only integers. When you do that, remember to change all the conditions with == to use the .equals method instead, for example:

public class ArraysDemo<T> {

    private final T[] values;
    private int size;

    // ...

    int indexOf(int value) {
        for (int i = 0; i < size; i++) {
            if (values[i].equals(value)) {
                return i;
            }
        }
        return -1;
    }
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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