Найти мин. 3-х цифр жестко закодированных

public static int min(int a, int b, int c)
{
    int result = 0 ;
    if( a < b && a < c && b < c) result = a ;
    else if( a < b && a < c && b > c) result = a ;

    else if( a > b && a < c && b < c) result = b ;
    else if( a < b && b  > c &&  c < a) result = c ;

    else if( a > b && b < c && a > c) result = b ;
    else if( a > b && a > c && c < b) result = c ;
    return result ;
}

Является ли это лучше, чем вложенные if заявления? Есть ли более читаемое решение , чем это? Для меня это выглядит довольно читаемо, но я не уверен, можно ли его улучшить.

31 голос | спросил ERJAN 1 PM00000040000004431 2014, 16:36:44

5 ответов


56
  

Мне это выглядит довольно читаемо

Мне это не нравится.


Ошибки:

Следующий метод вызывает все неправильно напечатать 0:

System.out.println(min(3, 2, 2));
System.out.println(min(3, 3, 3));
System.out.println(min(1, 3, 3));
System.out.println(min(4, 2, 4));

Это потому, что, когда вы смотрите на свой исходный код, это слишком сложно.

if( a < b && a < c && b < c) result = a ;
else if( a < b && a < c && b > c) result = a ;

Действительно ли имеет значение, если b < c или b > c? Нет, этого нет. И если b == c, то ни один из этих текущих не будет истинным, который возвращает return 0. Итак, это гигантская ошибка, ожидающая своего появления.

Итак, эти два первых, если нужно укоротить:

if (a <= b && a <= c) return a;

Обратите внимание, что я использую <= здесь, так что случай всех трех, имеющих одинаковое значение, обрабатывается правильно. Теперь также есть раннее возвращение, поэтому нам не нужны все эти else.


Если мы группируем остальные ваши утверждения в соответствии с тем, что они возвращают, мы имеем для return b:

else if( a > b && a < c && b < c) result = b ;
else if( a > b && b < c && a > c) result = b ;

Что, если мы всегда используем b в качестве первого операнда и полностью игнорируем, есть ли a < c или a > c (и опять же, a == c здесь не обрабатывается):

if (b <= a && b <= c) return b;

Выполнение того же для c:

if (c <= a && c <= b) return c;

В качестве одного из a, b или c действительно имеет , вы можете создать исключение если ни один из них:

public static int min(int a, int b, int c) {
     if (a <= b && a <= c) return a;
     if (b <= a && b <= c) return b;
     if (c <= a && c <= b) return c;
     throw new AssertionError("No value is smallest, how did that happen?");
}

Или, если вам не нравится это исключение:

public static int min(int a, int b, int c) {
     if (a <= b && a <= c) return a;
     if (b <= a && b <= c) return b;
     return c;
}

Это, на мой взгляд, значительно лучше, чем ваша оригинальная версия.

Однако я бы рекомендовал решение Pimgd, либо массив, либо используя цепочку Math.min.

ответил Simon Forsberg 1 PM00000050000002831 2014, 17:05:28
74

Для этого мы имеем java.lang.Math:

public static int min(final int a, final int b, final int c){
    return Math.min(a, Math.min(b, c));
}

Ничего себе, посмотри, как это коротко!

Но сегодня это 3 цифры, завтра 10.
Как альтернатива, как насчет массива?

public static int min(int... numbers){
    if(numbers.length == 0){
        throw new IllegalArgumentException("Can't determine smallest element in an empty set");
    }
    int smallest = numbers[0];
    for(int i = 1; i < numbers.length; i++){
        smallest = Math.min(smallest, numbers[i]);
    }
    return smallest;
}

Я бы использовал решение java.lang.Math, оно очень короткое и очень читаемое.

ответил Pimgd 1 PM00000040000000231 2014, 16:55:02
16

Я бы предложил использовать тернарный оператор, который вполне читаем для меня:

public static int min(int a, int b, int c) {
     return (a < b) ? (a < c) ? a
                              : c
                    : (b < c) ? b
                              : c;
}

Однако, если вы предпочитаете ifs:

public static int min(int a, int b, int c) {
    int min = a;
    if (min > b) min = b;
    if (min > c) min = c;
    return min;
}

В обоих случаях будут выполняться только два сравнения.

ответил Kao 1 PM00000040000005731 2014, 16:43:57
11

Вот моя идея: используйте необязательный параметр и используйте встроенные библиотеки. Гораздо понятнее и понятнее. Это работает с 3 параметрами, но также работает с любыми # параметрами.

public static Integer min(Integer... numbers) {
    if (numbers.length == 0) return 0;
    // wanted this: assert numbers.length > 0;, but does not work
    return Collections.min(Arrays.asList(numbers));
}

Вызов с помощью:

System.out.println(min(2,1,3)); 

дает 1.

Я попытался найти решение с использованием дженериков, но не могу заставить его работать.

Изменить : используйте IllegalArgumentException (спасибо @ SimonAndrà © Forsberg!):

public static Integer min(Integer... numbers) {
    if (numbers.length == 0) throw new IllegalArgumentException("Cannot have 0 arguments, i.e. min()");
    return Collections.min(Arrays.asList(numbers));
}

Изменить . Причина, по которой это решение работает, заключается в том, что Integer... numbers позволяет программе, вызывающей ее указать любое количество аргументов (даже 0) и внутри min здесь, он рассматривается как массив, который мы можем найти минимум этого массива, используя Collections.min и Arrays.asList.

ответил Ryan 1 PM00000060000004931 2014, 18:54:49
10

Я не думаю, что это действительно читаемо, поскольку оно передает много логики, которая просто не нужна.

Также вы должны, согласно большинству обычных стандартов кодирования Java, не размещать { в своей новой строке, также не должно быть пробелов между result и ; в ;

Я бы предложил следующее, хотя ему нужна Java 8:

return result ;

Таким образом, вы покрываете следующее:

  • Вы разрешаете произвольное количество целых чисел для передачи методу.
  • Вы получаете код public static int min(final int first, final int... others) { return IntStream.concat(IntStream.of(first), IntStream.of(others)) .min() .getAsInt(); } в ясном виде, используя потоки Java 8, он как-то проверяет минимум.
  • Поскольку вы знаете, что у вас есть хотя бы один результат, вы можете безопасно получить значение из min().

Один незначительный nitpick состоит в том, что OptionalInt для создания IntStream.concat(IntStream.of(first), IntStream.of(others)) довольно уродлив.

ответил skiwi 1 PM000000110000003231 2014, 23:40:32

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132