current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 7 down vote favorite

I'd like this code to be improved.

import java.util.Scanner;

public class ArrayElementFind {

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

        String[] arr= { "A","B","C","D"};

        Scanner in=new Scanner(System.in);

        System.out.println("Enter the char to search");
        String a=in.nextLine();

        //int len=arr.length;

        boolean check=loopcheck(arr,a);

        if(check){
            System.out.println("Element is present");
        }
        else{
            System.out.println("Element is not present");
        }


    }

    private static boolean loopcheck(String[] arr, String a) {
        // TODO Auto-generated method stub

        for(String s:arr){
            if(s.equals(a)){
                return true;
            }
        }

        return false;
    }
}
share|improve this question

2 Answers 2

up vote 6 down vote accepted

  • Since you're not using the try-with-resources statement with Scanner, you must close it at the end of main() to avoid leaking resources:

    in.close();

  • You should consider converting a to uppercase, in case the input is in lowercase.

    I would also rename a to something like guess. The name a says nothing about this variable, and it's generally discouraged to use single-letter variable names.

    String guess = in.nextLine().toUpperCase();

    The renaming also applies to arr. You could name it something like elemsToCheck.

share|improve this answer
1  
Thanks :) Great suggestions :) I love this community :) –  Arun Prakash Jul 19 '14 at 14:33
up vote 6 down vote

Minor nitpick that wasn't touched on: The assignments could use a little breathing space around the = for better readability. There's also inconsistent spacing between the brackets. 

    String[] arr= { "A","B","C","D"};

Becomes

    String[] arr = {"A","B","C","D"};

And

    Scanner in=new Scanner(System.in);

Becomes

    Scanner in = new Scanner(System.in);

Etc.

The check Boolean is unneeded. If you were calling loopcheck() more than once, I would say to leave it, but you're not. So don't. 

    if(loopcheck(arr,a)){
        System.out.println("Element is present");
    }
    else{
        System.out.println("Element is not present");
    }

You should also remove these comments. You've filled in the code already. They're just noise, not just in the code, but in your IDE's ToDo list as well. 

// TODO Auto-generated method stub
share|improve this answer
    
Thanks :) Great suggestions :) I love this community :) –  Arun Prakash Jul 19 '14 at 14:32
    
So do I! You're welcome and welcome to Code Review. –  RubberDuck Jul 19 '14 at 15:17

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.