Избавьтесь от уродливых утверждений if

Как насчет такого подхода:

int getSize(int v) {
    int[] thresholds = {145, 117, 68, 51, 22, 10};

    for (int i = 0; i < thresholds.length; i++) {
        if (v > thresholds[i]) return i+1;
    }
    return 1;
}

---

Функционально: (продемонстрировано в Scala)

def getSize(v: Int): Int = {
  val thresholds = Vector(145, 117, 68, 51, 22, 10)
  thresholds.zipWithIndex.find(v > _._1).map(_._2).getOrElse(0) + 1
}

Используя API NavigableMap:

NavigableMap<Integer, Integer> s = new TreeMap<Integer, Integer>();
s.put(10, 6);
s.put(22, 5);
s.put(51, 4);
s.put(68, 3);
s.put(117, 2);
s.put(145, 1);

return s.lowerEntry(v).getValue();

Самая очевидная проблема с решением OPs-ветвление, поэтому я бы предложил полиномиальную регрессию. Это приведет к хорошему выражению без ветвей на форме

size = round(k_0 + k_1 * v + k_2 * v^2 + ...)

Вы, конечно, не получите точного результата, но если вы можете терпеть некоторые отклонения, это очень эффективная альтернатива. Поскольку "оставить неизмененным" поведение исходной функции для значений, где v<10 невозможно смоделировать с помощью полинома, я взял на себя смелость предположить интерполяцию удержания нулевого порядка для этого региона.

Для 45-градусного полинома со следующими коэффициентами,

-9.1504e-91 1.1986e-87 -5.8366e-85 1.1130e-82 -2.8724e-81 3.3401e-78 -3.3185e-75  9.4624e-73 -1.1591e-70 4.1474e-69 3.7433e-67 2.2460e-65 -6.2386e-62 2.9843e-59 -7.7533e-57 7.7714e-55 1.1791e-52 -2.2370e-50 -4.7642e-48 3.3892e-46 3.8656e-43 -6.0030e-41 9.4243e-41 -1.9050e-36 8.3042e-34 -6.2687e-32 -1.6659e-29 3.0013e-27 1.5633e-25 -8.7156e-23  6.3913e-21 1.0435e-18 -3.0354e-16 3.8195e-14 -3.1282e-12 1.8382e-10 -8.0482e-09 2.6660e-07 -6.6944e-06 1.2605e-04 -1.7321e-03 1.6538e-02 -1.0173e-01 8.3042e-34 -6.2687e-32 -1.6659e-29 3.0013e-27 1.5633e-25 -8.7156e-23 6.3913e-21 1.0435e-18 -3.0354e-16 3.8195e-14 -3.1282e-12 1.8382e-10 -8.0482e-09 2.6660e-07 -6.6944e-06 1.2605e-04 -1.7321e-03 1.6538e-02 -1.0173e-01 3.6100e-01 -6.2117e-01 6.3657e+00

, Вы получаете красиво подогнанную кривую:

И, как вы можете видеть, вы получаете 1-нормальную ошибку всего 1,73 во всем диапазоне от 0 до 200*!

*результаты для v∉[0,200] могут отличаться.

return v > 145 ? 1 
     : v > 117 ? 2 
     : v > 68 ? 3 
     : v > 51 ? 4 
     : v > 22 ? 5 
     : v > 10 ? 6 
     : "put inital size value here";

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

if ( v > 145 ) return 1;
if ( v > 117 ) return 2;
if ( v >  68 ) return 3;
if ( v >  51 ) return 4;
if ( v >  22 ) return 5;
if ( v >  10 ) return 6;
return ...;     // The <= 10 case isn't handled in the original code snippet. 

Смотрите обсуждение множественного возврата или нет в org.жизнь.ответ java .

Здесь есть тонна ответов и предложений, но я честно не вижу ни одного из них "красивее" или "элегантнее", чем оригинальный метод.

Если бы у вас были десятки или сотни итераций для проверки, то я мог бы легко увидеть, как вы переходите к циклу for, но, честно говоря, для нескольких сравнений, которые у вас были, придерживайтесь if и двигайтесь дальше. Это не так уж уродливо.

return (v-173) / -27;

Вот мой шанс на это...

Обновление: Исправлено. Предыдущее решение давало неверные ответы на точные значения (10,22,51...). Этот параметр по умолчанию равен 6 для if val

   static int Foo(int val)
    {
                          //6, 5, 4, 3, 2 ,1
        int[] v = new int[]{10,22,51,68,117,145};
        int pos = Arrays.binarySearch(v, val-1);
        if ( pos < 0) pos = ~pos;
        if ( pos > 0) pos --;
        return 6-pos;
    }

У меня есть еще одна версия для вас. Я на самом деле не думаю, что это лучший вариант, потому что он добавляет ненужную сложность во имя "производительности", когда я на 100% уверен, что эта функция никогда не будет Боровом производительности (если только кто-то не вычисляет размер в узком цикле миллион раз ...).

Но я представляю его только потому, что я думал, что выполнение жестко закодированного двоичного поиска будет своего рода интересным. Он не выглядит очень бинарным, потому что там недостаточно элементов, чтобы идти очень глубоко, но он имеет то достоинство, что он возвращает результат не более чем в 3 тестах, а не в 6, как в исходном посте. Операторы return также упорядочены по размеру, что поможет в понимании и/или модификации.

if (v > 68) {
   if (v > 145) {
      return 1
   } else if (v > 117) {
      return 2;
   } else {
      return 3;
   }
} else {
   if (v > 51) {
      return 4;
   } else if (v > 22) {
      return 5;
   } else {
      return 6;
   }
}

7 - (x>10 + x>22 + x>51 + x>68 + x>117 + x>145)

Где 7 - значение по умолчанию (x <= 10).

Edit: изначально я не понимал, что этот вопрос касается Java. Это выражение недопустимо в Java, но допустимо в C/C++. Я оставлю ответ, так как некоторые пользователи сочли его полезным.

Моя способность комментировать еще не включена, надеюсь, никто не скажет "по праву", основываясь на моем ответе...

Приукрашивание уродливого кода может / должно быть определено как попытка достичь:

1. читабельность (ОК, констатация очевидного-возможно, излишняя для вопроса)
2. Производительность - в лучшем случае ищет оптимальную, в худшем-это не большой Сток
Прагматизм-это не так далеко от того, как большинство людей делают вещи, учитывая обычную проблему, которая не нуждается в элегантном решении. или уникальное решение, изменение его позже должно быть естественным усилием, не требующим много воспоминаний.

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

Просматривая все комментарии на эту тему, на момент моего написания, оказывается, что только орг.жизнь.java подняла вопрос производительности (и, возможно, mfloryan тоже, заявив что-то будет "длиннее"). Конечно, в большинстве ситуаций, и учитывая этот пример, он не должен нести заметного замедления, как бы вы его ни писали.

Однако, вложив свои условия и оптимально упорядочив условия, можно улучшить производительность [стоит, особенно если это было зациклено].

Все сказанное, условия вложенности и упорядочения (которые более сложны, чем ваш пример), вызванные решимостью достичь как можно более быстрого выполнения, часто приводят к тому, что менее читаемый код, и код, который труднее изменить. Я снова ссылаюсь на № 3, прагматизм... уравновешивание потребностей.

Вот объектно-ориентированное решение, класс под названием Mapper<S,T>, который отображает значения из любого типа, реализующего сопоставимый с любым целевым типом.

Синтаксис:

Mapper<String, Integer> mapper = Mapper.from("a","b","c").to(1,2,3);

// Map a single value
System.out.println(mapper.map("beef")); // 2

// Map a Collection of values
System.out.println(mapper.mapAll(
    Arrays.asList("apples","beef","lobster"))); // [1, 2, 3]

Код:

public class Mapper<S extends Comparable<S>, T> {

    private final S[] source;
    private final T[] target;

    // Builder to enable from... to... syntax and
    // to make Mapper immutable
    public static class Builder<S2 extends Comparable<S2>> {
        private final S2[] data;
        private Builder(final S2[] data){
            this.data = data;
        }
        public <T2> Mapper<S2, T2> to(final T2... target){
            return new Mapper<S2, T2>(this.data, target);
        }
    }


    private Mapper(final S[] source, final T[] target){
        final S[] copy = Arrays.copyOf(source, source.length);
        Arrays.sort(copy);
        this.source = copy;
        this.target = Arrays.copyOf(target, target.length);
    }

    // Factory method to get builder
    public static <U extends Comparable<U>, V> Builder<U> from(final U... items){
        return new Builder<U>(items);
    }

    // Map a collection of items
    public Collection<T> mapAll(final Collection<? extends S> input){
        final Collection<T> output = new ArrayList<T>(input.size());
        for(final S s : input){
            output.add(this.map(s));
        }
        return output;
    }

    // map a single item
    public T map(final S input){
        final int sourceOffset = Arrays.binarySearch(this.source, input);
        return this.target[
            Math.min(
                this.target.length-1,
                sourceOffset < 0 ? Math.abs(sourceOffset)-2:sourceOffset
            )
        ];
    }
}

Edit: наконец-то заменил метод map() на более эффективную (и более короткую) версию. Я знаю: версия, которая ищет разделы, все равно будет быстрее для больших массивов, но извините: я слишком ленив.

Если вы считаете, что это слишком раздуто, подумайте это:

1. он содержит конструктор, который позволяет создавать Сопоставитель с использованием синтаксиса varargs. Я бы сказал, что это необходимо для удобства использования
2. он содержит как один элемент, так и метод отображения коллекции
3. он неизменяем и, следовательно, потокобезопасен

Конечно, все эти функции можно было бы легко удалить, но код был бы менее полным, менее удобным или менее стабильным.

Есть ли лежащее в основе этого математическое правило? Если это так, вы должны использовать это: но только если это исходит из области проблем, а не просто какая-то формула, которая случайно подходит к случаям.

int[] arr = new int[] {145, 117, 68, 51, 22, 10};
for(int index = 0; index < arr.length; index++)
{
  if(v > arr[index]) return 1 + index; 
}

return defaultValue;

Вы можете переписать его в ARM-коде. Это всего лишь 7 циклов в худшем случае и стройные 164 байта. Надеюсь, это поможет. (Примечание: это непроверено)

; On entry
;   r0 - undefined
;   r1 - value to test
;   lr - return address
; On exit
;   r0 - new value or preserved
;   r1 - corrupted
;
wtf
        SUBS    r1, r1, #10
        MOVLE   pc, lr
        CMP     r1, #135
        MOVGT   r0, #1
        ADRLE   r0, lut
        LDRLEB  r0, [r0, r1]
        MOV     pc, lr
;
; Look-up-table
lut
        DCB     0   ; padding
        DCB     6   ; r1 = 11 on entry
        DCB     6
        DCB     6
        DCB     6
        DCB     6
        DCB     6
        DCB     6
        DCB     6
        DCB     6
        DCB     6
        DCB     6
        DCB     6
        DCB     5   ; r1 = 23 on entry
        DCB     5
        ...
        ALIGN

Просто для полноты, позвольте мне предложить, что вы могли бы настроить размеры массива с 145 элементами, так что ответ может быть возвращен непосредственно в виде размеров[v]. Прошу прощения, что не записал всего этого. Конечно, вы должны были убедиться, что v находится в пределах досягаемости.

Единственная причина, по которой я могу так поступить, - это если вы собираетесь создать массив один раз и использовать его тысячи раз в приложении, которое должно быть очень быстрым. Я упоминаю его в качестве примера компромисс между памятью и скоростью (не та проблема, которая была когда-то), а также между временем настройки и скоростью.

На самом деле, если размеры, вероятно, изменятся, это может быть хорошей альтернативной стратегией:

CREATE TABLE VSize (
   LowerBound int NOT NULL CONSTRAINT PK_VSize PRIMARY KEY CLUSTERED,
   Size int NOT NULL
)
INSERT VSize VALUES (10, 6)
INSERT VSize VALUES (22, 5)
INSERT VSize VALUES (51, 4)
INSERT VSize VALUES (68, 3)
INSERT VSize VALUES (117, 2)
INSERT VSize VALUES (145, 1)

И хранимая процедура или функция:

CREATE PROCEDURE VSizeLookup
   @V int,
   @Size int OUT
AS
SELECT TOP 1 @Size = Size
FROM VSize
WHERE @V > LowerBound
ORDER BY LowerBound

Очевидный ответ-использовать Groovy:

def size = { v -> [145,117,68,51,22,10].inject(1) { s, t -> v > t ? s : s + 1 } }

Одни лайнеры всегда лучше. Возвращает 7 для неопределенного случая, где v

Почему кто-то не предложил оператор switch. это гораздо лучше, чем если бы еще лестница.

public int getSize(int input)
    {
        int size = 0;
        switch(input)
        {
        case 10:
            size = 6;
            break;

        case 22:
            size = 5;
            break;


        case 51:
            size = 4;
            break;

        case 68:
            size = 3;
            break;

        case 117:
            size = 2;
            break;

        case 145:
            size = 1;
            break;
        }

        return size;
    }

Это мой пример кода, использующий SortedSet. Вы инициализируете границы один раз.

SortedSet<Integer> boundaries = new SortedSet<Integer>;

boundaries.add(10);

boundaries.add(22);

boundaries.add(51);

boundaries.add(68);

boundaries.add(117);

boundaries.add(145);

Затем используйте его таким образом для нескольких значений v (и инициализированного размера)

SortedSet<Integer> subset =  boundaries.tailSet(v);
if( subset.size() != boundaries.size() )
  size = subset.size() + 1;

Интересно, что существует множество красивых ответов на простой "уродливый" вопрос. Мне больше всего нравится ответ mfloryan, однако я бы продвинул его дальше, удалив жестко закодированный массив внутри метода. Что-то вроде:

int getIndex(int v, int[] descArray) {
    for(int i = 0; i < descArray.length; i++)
        if(v > descArray[i]) return i + 1;
    return 0;
}

Теперь он становится более гибким и может обрабатывать любой заданный массив в порядке убывания, и метод найдет индекс, к которому принадлежит значение 'v'.

ПС. Я пока не могу комментировать ответы.

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

final int minBoundary = 10;
final int maxBoundary = 145;
final int maxSize = 6;
Vector<Integer> index = new Vector<Integer>(maxBoundary);
    // run through once and set the values in your index

Впоследствии

if( v > minBoundary )
{
   size = (v > maxBoundary ) ? maxSize : index[v];
}

То, что мы делаем здесь, - это помечаем все возможные результаты v в пределах диапазона и где они падают, а затем нам нужно только проверить граничные условия.

Проблема с этим заключается в том, что он использует больше памяти и, конечно, если maxBoundary намного больше, это будет очень неэффективно (а также займет больше времени время инициализации).

Иногда это может быть лучшим решением для данной ситуации.

            if (v <= 10)
                return size;
            else {
                size = 1;

                if (v > 145)
                    return size;
                else if (v > 117)
                    return ++size;
                else if (v > 68)
                    return (size+2);
                else if (v > 51)
                    return (size+3);
                else if (v > 22)
                    return (size+4);
                else if (v > 10)
                    return (size+5);
            }

При этом будут выполняться только необходимые операторы if.

Еще одна вариация (менее выраженная, чем ответ Джорджа )

  //int v = 9;
  int[] arr = {145, 117, 68, 51, 22, 10};
  int size = 7; for(;7 - size < arr.length && v - arr[size - 2] > 0; size--) {};
  return size;