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 2 down vote favorite

I wrote this program for sorting an array of type String alphabetical without using compareTo() method so I am saying that if you can find errors in my program please tell me I will try to fix it.

class some{
 public static void main(String[] args)  {
  String array[]={"names here"};
  System.out.print("before:");
 for (int    i=0 ;i<array.length;i++ ) 
    System.out.print(array[i]+" ");

 array  = SortA(array);
      System.out.println();
           System.out.print("after:");
     for (int    i=0 ;i<array.length;i++ ) 
      System.out.print(array[i]+" ");


}//end of main method

    public static int getSmallestRange(String[] array){
        int range = array[0].length()-1;
        for (int i =0;i<array.length ;i++ ) {

            if(range>array[i].length()-1){
                range = array[i].length()-1;

            }

        }
        return range ;
}
  public static String[] SortA(String [] array){
         String tmp="";
  int index = 0 ;
  for (int i =0;i<array.length ;i++ ) 
        for (int j = i+1;j<array.length ;j++ ) {
              if(array[i].charAt(index)>array[j].charAt(index)){
                          tmp = array[i];
                          array[i] = array[j];
                          array[j] = tmp ;
              }//end of if
        }//end of loop
        index++;

        for (int x =0;x<array.length-1 ;x++ ){
              for (int y =x+1;y<array.length ;y++ ) {


              if(array[x].charAt(0)==array[y].charAt(0)){

                    if(array[x].charAt(index)>array[y].charAt(index)){
                          tmp = array[x];
                          array[x] = array[y];
                          array[y] = tmp ;

                    }

                    else if(array[x].charAt(index)==array[y].charAt(index)){
                    if(index<getSmallestRange(array))
                          index++;
                    }

              }//end of if
        }//end of loop
  }

  return array;
 }//end of method
}
share|improve this question
    
Currently it's incorrect, given an initial array of {"dds", "dda"}, it fails to sort them properly. –  bowmore Jun 2 '13 at 6:50
2  
First off, you have to properly format your code. The indentation is off and very confusing. –  Michael Zedeler Jun 2 '13 at 10:48
    
@MichaelZedeler: I think checking and correcting usage of whitespace is appropriate for a code review. I mean that no properly formatted code in here is not a "problem", as we are here to point these things out. Just pointing that out because you said "first off, you have to..." which makes it sound like it is a criteria (which I disagree with). –  Bobby Jun 2 '13 at 18:14
1  
In this context, I mean that formatting is first priority over everything else. Learning to program requires that you read and re-read your own code over and over again, so not formatting it right is really getting in the way of ever getting better. –  Michael Zedeler Jun 2 '13 at 18:19
    
@MichaelZedeler: I completely agree with that! –  Bobby Jun 2 '13 at 19:59

1 Answer 1

up vote 1 down vote

First off, your use of whitespace is a mess, fix it. Press "Format Code" in your favorite IDE or do it manually.

class some{
  • The Java Naming Conventions say that class names should be UpperCamelCase.
  • Use a visibility modificator for clearness (private, public).
String array[]={"names here"};

Use descriptive variable names, like sourceArray or unsortedArray.

for (int    i=0 ;i<array.length;i++ )
  • Use appropriate variable names. This is a little bit unpopular with most people, but I prefer to use descriptive names even for such simple loops (int idx).

  • As said before, your use pf whitespace is very confusing:

    for (int idx = 0; idx < array.length; idx++) {

  • Use appropriate loops, in this case a for-each loop:

    for (String item : array) {

String tmp="";
int index = 0 ;

Declaring variables once and reusing them through out whole function, especially larger functions, is bad. Declare variables where you need them and name them appropriately, like this:

String swapTemp = array[i];
array[i] = array[j];
array[j] = swapTemp;

       }//end of if
 }//end of loop

If code is correctly formatted, short and precise (only do one function per function), such comments are absolutely unnecessary.

Consider using the Java Documentation feature.

share|improve this answer
    
thank you guys i will try –  Aram Rafeq Jun 3 '13 at 11:57

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.