2014-11-21 86 views
-3

只是想知道如果我使用嵌套的if语句太多。我一直在环顾四周,似乎人们试图不使用它们。代码也看起来杂乱无章?无论如何它是这样的:我的代码是草率/糟糕吗?

import java.util.Arrays; 

public class Main { 

private String user_input = ""; 
private int max_score = 6; 
private int sum; 

private void check_scores(String scores){ 
    user_input = scores; 
    String[] temp; 

    // Check if user_input is valid 
    //^Match with beginning of line | [0-9] Allow 0-9 | , Allow comma | + Match one or more | $ Match End of line 
    if (user_input.matches("^[0-9,]+$")) { 

     // Check if string starts with an , 
     if(user_input.charAt(0) == ',') { 
      // If it does parse and substring to remove them 
      // otherwise the following regex leaves one behind 
      int i = 0; 
      while (!Character.isDigit(user_input.charAt(i))) i++; 
      int j = user_input.length(); 
      user_input = user_input.substring(i,j); 
     } 

     // (.) Match any character) | \1 If it is followed by itself | + Match one or more | $1 replace by the first captured char. 
     user_input = user_input.replaceAll("(.)\\1+", "$1"); 

     System.out.println(user_input); 

     // Split at the ',' and put each number in it's own cell in the array 
     temp = user_input.split(","); 
     System.out.println(Arrays.toString(temp)); 

     // Check if temp is equal to max_scores 
     if (temp.length == max_score){ 
      int[] ui_array = new int[temp.length]; 

      // Parse String[] into int[] 
      for (int i = 0; i < temp.length; i++){ 
       try { 
        ui_array[i] = Integer.parseInt(temp[i]); 
       } catch (NumberFormatException nfe) {}; // If triple checking isn't enough... 
      } 
      System.out.println("temp array(String): " + Arrays.toString(temp)); 
      System.out.println("ui_array(int): " + Arrays.toString(ui_array)); 

      // Add up all elements in ui_array 
      for (int j = 0 ; j < ui_array.length; j++) { 
       sum += ui_array[j]; 
      } 
      System.out.println("Scores sum:" + sum + " Number of scores:" + ui_array.length + " Number of ends:" + ui_array.length/6); 
     } 
     else { 
      System.out.println("You have " + temp.length + " scores. Acceptable amount is " + max_score); 
     } 
    } 
    else { 
     System.out.println("Invalid Input. (Only #'s and ,'s allowed)"); 
    } 
} 

public static void main(String[] args) { 
    Main main = new Main(); 
    main.check_scores("1,M,7,10,,4,8,"); 
    main.check_scores("1,6,7,10,,4,8,,,,,,,1,2,6,10,2,10"); 
    main.check_scores(",,,,,,,1,2,6,10,2,10"); 
    main.check_scores("10,2,1,5,7,1"); 
    main.check_scores("6,2, ,,5,6,1"); 
    } 
} 

我刚才一直想知道一段时间人们怎么想我如何去做事情。

+8

这种问题更适合[CodeReview.SE](http://codereview.stackexchange.com/)。 – irrelephant 2014-11-21 05:25:09

+0

考虑重构该方法。 – 2014-11-21 05:26:44

+0

为什么重命名'scores'?如果你想叫它'user_input',只需重命名参数即可。此外,你应该在你的条件之后做一个新的线。您还应该将temp重命名为有意义的内容;我使用temp的唯一时间就是它仅仅用于几行中的小事,但你使用它很多,所以它应该有一个描述性的名字。我低估了这个问题,因为就像不像大象说的那样,这是CodeReview Stack Exchange的用处。 – 2014-11-21 05:43:37

回答

2

有几件事情我想指出:

  1. 就个人而言,我认为像

    的public void方法()
    {

    }

是多少比

更具可读性
public void method() { 

} 

特别是当你有包含其他结构的方法时。我敢肯定,有些人可能不介意,但我从来没有听说有人说第一张不可读,而很多人抱怨第二张。对不起,第一个没有正确格式,但SO不会允许它..看起来网站管理员不同意我这一个。

  1. 这是标准变量,如someVariableName而不是some_variable_name。第一个词应该是小写字母,其他所有字母都应该是连续的。方法也是如此。

  2. 在你check_scores(String)方法你有user_input = scores;,但您的实现,没有必要一个全局变量或传递变量指定的其他地方,所以这是浪费内存。实际上,因为你的类变量没有在这个方法的范围之外使用,所以你应该在方法中声明它们全部。我知道这是一个微不足道的例子,但为了符合面向对象的编程思想,你的Main类可能应该放在一个单独的文件中,并且可以通过在驱动程序类的main方法中创建一个对象来运行代替。

而且,正如你提到的,嵌套几个语句,如果没有必要,可以马虎,因为它会很快变得难以阅读和遵守。

+1

不同意第一点 – tomasb 2014-11-21 05:44:55