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

есть такой код:

for (ListIterator<Double> it = list.listIterator(); it.hasNext(); ) {
    Double value = it.next();
    Double refValue = null;
    if (value != 0) {
        if (value < 0 && it.hasNext()) {
            refValue = it.next();
            it.previous();
        } else if (value > 0 && it.hasPrevious()) {
            it.previous();
            refValue = it.previous();
            it.next();
            it.next();
        }
    }
    ... //какие-то операции с value и refValue
}

што он делает — проходится по отсортированному списку из чисел (вида -100, -90, -2, 0, 3, 50) , и для каждого из чисел (кроме центрального — 0) находит "пару" — в зависимости оттого если число отрицательное или положительное — это или следующее число в списке, либо предыдущее. ноля быть не может. зачем такое надо не спрашивайте !!

вопрос — как переписать этот уродский код чтоб было красивее. одобряется использование фич джавы 8 — если они тут подойдут

спасибо

Здравствуйте, зиг, Вы писали:

зиг>вопрос — как переписать этот уродский код чтоб было красивее. одобряется использование фич джавы 8 — если они тут подойдут
java8 стримы работают с текущим значением, там нет "предыдущего" и "следующего", так, что я бы даже отказался от итераторов. Не сказал бы, что короче, но на мой вкус читабельнее:

public static void main(String... args) {
    List<Integer> ints = Arrays.asList(-100, -90, -2, 0, 3, 50);
    for(int i = 0; i != ints.size(); ++i) {
        Integer value = ints.get(i);
        Integer refVal = getRefVal(value,
                i != 0 ? ints.get(i-1) : null,
                i != ints.size()-1 ? ints.get(i+1) : null);
        System.out.println("value: " + value + ", refVal: " + refVal);
    }
}

private static Integer getRefVal(Integer cur, Integer prev, Integer next) {
    Integer result = null;
    if ( cur < 0 &&  next != null ) {
        result = next;
    } else if ( cur > 0 && prev != null ) {
        result = prev;
    }
    return result;
}

Здравствуйте, зиг, Вы писали:

У меня такое вырисовалось:

  private void processList(List<Double> list) {
        int zeroIndex = findZero(list);
        for (int i = 1; i < zeroIndex; i++) {
            Double value = list.get(i);
            Double refValue = list.get(i - 1);
            process(value, refValue);
        }
        for (int i = zeroIndex + 1; i < list.size() - 1; i++) {
            Double value = list.get(i);
            Double refValue = list.get(i + 1);
            process(value, refValue);
        }         
    }

    private void process(Double value, Double refValue) {
        //какие-то операции с value и refValue
    }

ЗЫ. Про надежность сравнения ==/!= double в курсе же?

import java.util.Arrays;
import java.util.List;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BiConsumer;
import java.util.stream.IntStream;

public class Pairs {
    public static void main(String[] args) {
        List<Double> list = Arrays.asList(-100.0, -90.0, -2.0, 0.0, 3.0, 50.0);
//        List<Double> list = Arrays.asList(-100.0, -90.0, -2.0, 0.0, 3.0, 50.0);
//        List<Double> list = Arrays.asList(-100.0, -90.0);
//        List<Double> list = Arrays.asList(3.0, 50.0);
        BiConsumer<Double, Double> action = (value, refValue) -> System.out.println(value+", "+refValue);

        {
            System.out.println("readability: good, performance 2xO(n) for array list, 2xO(n^2) for linked list");
            IntStream
                .range(0, list.size())
                .filter(idx -> list.get(idx) < 0)
                .forEach(idx -> action.accept(list.get(idx), getIf(list, idx+1)));
            IntStream
                .range(0, list.size())
                .filter(idx -> list.get(idx) > 0)
                .forEach(idx -> action.accept(list.get(idx), getIf(list, idx-1)));
        }

        {
            System.out.println("readability: fair, performance O(n) for array list, O(n^2) for linked list");
            IntStream
                .range(0, list.size())
                .filter(idx -> list.get(idx) != 0)
                .forEach(idx -> { double v = list.get(idx); action.accept(v, getIf(list, idx + (v < 0 ? 1 : -1)));});
        }
        {
            System.out.println("readability: bad, performance O(n) for all lists");
            AtomicReference<Double> prev = new AtomicReference<>();
            list.stream().forEach(n -> {
                Double p = prev.getAndSet(n);
                if (n > 0) action.accept(n, p);
                else if (p != null && p < 0) action.accept(p, n);
            });
            if (prev.get() != null && prev.get() < 0) action.accept(prev.get(), null);
        }
    }

    private static Double getIf(List<Double> list, int index) {
        return (index < 0 || index >= list.size()) ? null : list.get(index);
    }
}

Ловко ты сделал вид, что списки всегда ArrayList

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

Ещё неплохой вариант с reduce, жалко последний элемент всё равно приходится отдельно и через жопу обрабатывать

        {
            System.out.println("readability: bad, performance O(n) for all lists");
            list.stream().reduce((p,n) -> {
                if (n > 0) action.accept(n, p);
                else if (p != null && p < 0) action.accept(p, n);
                return n;
            }).ifPresent(p -> {if (p < 0) action.accept(p, null);});
        }

Здравствуйте, зиг, Вы писали:

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

import java.util.Arrays;
import java.util.Iterator;
import java.util.List;

public class PrevNext {
    public static void main(String[] args) {
        //Prepare data
        List<Double> list = Arrays.asList(-100.0, -90.0, -2.0, 0.0, 3.0, 50.0, null, null);
        Iterator<Double> it = list.iterator();
        //Init loop
        Double value = it.next();
        Double next = it.next();
        Double previous = null;
        //Iterate and process
        while (value != null){
            Double ref = null;
            if(value > 0) ref = next;
            if(value < 0) ref = previous;

            process(value, ref);
            
            previous = value;
            value = next;
            next = it.next();
        }
    }

    private static void process(Double value, Double ref) {
        System.out.print(value);
        System.out.print(":");
        System.out.println(ref);
    }
}

Можно ещё попробовать троицу заменить на очередь в три элемента.

Здравствуйте, зиг, Вы писали:

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

ИМХО из всех предложенных вариантов твой — самый читабельный.
Оставь как есть.

Java style. Городим новый класс чтобы сэкономить пару строк клиентского кода.

import java.util.*;

public class PrevNext {
    public static void main(String[] args) {
        //Prepare data
        List<Double> list = Arrays.asList(-100.0, -90.0, -2.0, 0.0, 3.0, 50.0, null, null);
        Iterator<Double> it = list.iterator();

        //Init loop
        Triad<Double> triad = new Triad<>(null, it.next(), it.next());
        //Iterate and process
        while (triad.current() != null){
            Double ref = null;
            if(triad.current() > 0) ref = triad.next();
            if(triad.current() < 0) ref = triad.previous();

            process(triad.current(), ref);

            triad.push(it.next());
        }
    }

    private static void process(Double value, Double ref) {
        System.out.print(value);
        System.out.print(":");
        System.out.println(ref);
    }

    private static class Triad<T> {

        private final T[] triad;

        public Triad(T ... triad) {
            this.triad = triad;
        }

        public T previous() {
            return triad[0];
        }

        public T current() {
            return triad[1];
        }

        public T next() {
            return triad[2];
        }

        public void push(T next) {
            System.arraycopy(triad, 1, triad, 0, 2);
            triad[3] = next;  
        }
    }
}

Здравствуйте, Stanislav V. Zudin, Вы писали:

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

Здравствуйте, зиг, Вы писали:

Для массива вообще всё просто:

import java.util.*;

public class PrevNext {
    public static void main(String[] args) {
        //Prepare data
        List<Double> list = Arrays.asList(null, -100.0, -90.0, -2.0, 0.0, 3.0, 50.0, null);
        for (int i = 1; i < list.size() - 1; i++) {
            Double value = list.get(i);
            Double ref = value == 0.0
                    ? null
                    : list.get((int) (i + Math.signum(value)));
            process(value, ref);
        }
    }

    private static void process(Double value, Double ref) {
        System.out.println(value + " : " + ref);
    }
}

Здравствуйте, bzig, Вы писали:

B>Ловко ты сделал вид, что списки всегда ArrayList
Правда ловко
тогда так:

public static void main(String... args) {
    List<Integer> ints = Arrays.asList(-100, -90, -2, 0, 3, 50);
    Iterator<Integer> it = ints.iterator();
    Integer prev = null;
    Integer value = it.next();
    Integer next = it.next();
    do {
        Integer refVal = getRefVal(value, prev, next);
        System.out.println("value: " + value + ", refVal: " + refVal);

        prev = value;
        value = next;
        next = it.hasNext() ? it.next() : null;
    } while ( value != null );
}

private static Integer getRefVal(Integer cur, Integer prev, Integer next) {
    Integer result = null;
    if ( cur < 0 &&  next != null ) {
        result = next;
    } else if ( cur > 0 && prev != null ) {
        result = prev;
    }
    return result;
}

Здравствуйте, зиг, Вы писали:

зиг>што он делает — проходится по отсортированному списку из чисел (вида -100, -90, -2, 0, 3, 50) , и для каждого из чисел (кроме центрального — 0) находит "пару" — в зависимости оттого если число отрицательное или положительное — это или следующее число в списке, либо предыдущее. ноля быть не может. зачем такое надо не спрашивайте !!

зиг>вопрос — как переписать этот уродский код чтоб было красивее. одобряется использование фич джавы 8 — если они тут подойдут

А так нельзя?

int ap=0; double[] ac=new double[2];
for (ListIterator<Double> it = list.listIterator(); it.hasNext(); ac[0]=ac[1]) {
    ac[ap]=it.next(); if (ap<1) { ap=1; continue; }
    if (ac[1]<0) output(ac[1],ac[0]);
    if (ac[0]>0) output(ac[0],ac[1]);
}

Здравствуйте, Blazkowicz, Вы писали:

B>Здравствуйте, Stanislav V. Zudin, Вы писали:

SVZ>>ИМХО из всех предложенных вариантов твой — самый читабельный.
SVZ>>Оставь как есть.
B>Читаемым он бы был, если бы не куча проверок на пограничные условия.
Меня скорее все эти next() & prev() в разных последовательностях заставляют немного напрячься.

Здравствуйте, зиг, Вы писали:

зиг>есть такой код:
Этот код, кстати, падает если входной массив содержит только положительные числа. Это так и должно быть??

Я бы написал тупо как есть:

    private static void doStuff(final List<Double> list) {
        Double previous = null;
        for (final ListIterator<Double> it = list.listIterator(); it.hasNext(); ) {
            final Double value = it.next();
            final Double refValue;
            if (value < 0) {
                refValue = peekNext(it);
            } else if (value > 0) {
                refValue = previous;
            } else {
                refValue = null;
            }
            previous = value;

    ... //какие-то операции с value и refValue
        }
    }

    private static Double peekNext(final ListIterator<Double> it) {
        if(!it.hasNext()) return null;
        final Double val = it.next();
        it.previous();
        return val;
    }

Этот код, кстати, не падает.

Здравствуйте, зиг, Вы писали:

Ну с'mon ... тривиальный sliding window же ж:

Double pv, cv;
for (ListIterator<Double> it = list.listIterator(); it.hasNext(); ) {
    pv = cv;
    cv = it.next();
    Double rv;    
    if (pv != null && cv != 0) {
        if (cv < 0)
          rv = pv;
        else if (cv > 0)
          rv = cv;
    }
    ... //какие-то операции с value и refValue
}

Здравствуйте, зиг, Вы писали:

Предполагаю, что у нас вначале не итератор, а коллекция, и дополнительно добавил граничные условия.

По существу здесь 4 метода:
1) преобразование из итератора в коллекцию с нормальными граничными условиями (в конце null чтоб не делать дополнительный if). Это не писал, ибо лень;
2) Из этой коллекции возвращаю пары value — refValue. В качестве пар заюзал обычный массив, в других языках вернул бы кортеж или пару как тип. Причем вернул как стрим, он ленивый;
3) Вычисление refvalue в отдельный метод вынес — тоже можно тестить отдельно;
3) собственно сама логика, которая выполняет сейчас только печать.

Строчек кода больше, но зато здесь следование single responsibility principle и каждый из методов можно тестить по отдельности, если б я сделал их публичными.

import java.util.*;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Stream;

public class Test {
    public static void main(String[] args) {
        //Это пример, в исходном примере там итератор, потому лучше из этого итератора собрать какой то Collection
        Collection<Double> list = Arrays.asList(-100.0, -90.0, -2.0, 0.0, 3.0, 50.0, null);
        Stream<Double[]> pairStream = getValueToRefValueStream(list);
        processStream(pairStream);
    }

    private static Stream<Double[]> getValueToRefValueStream(Collection<Double> list) {
        Iterator<Double> it = list.iterator();
        if (!it.hasNext()) {
            return Stream.empty();
        }
        final AtomicReference<Double> pvRef = new AtomicReference<>(null);
        final AtomicReference<Double> cvRef = new AtomicReference<>(it.next());

        Stream<Optional<Double[]>> optionalStr = list
                .stream()
                .skip(1)
                .map( (Double plus1) -> {
            Double minus1 = pvRef.get();
            Double value = cvRef.get();
            pvRef.set(cvRef.get());
            cvRef.set(plus1);
            Double refValue = calculateRefValue(plus1, minus1, value);
                    if (refValue != null) {
                return Optional.of(new Double[]{value, refValue});
            }
            return Optional.empty();
        });
        return optionalStr.filter(Optional::isPresent).map(Optional::get);
    }

    private static Double calculateRefValue(Double plus1, Double minus1, Double value) {
        Double result = null;
        if (value != null && value != 0) {
            if (value < 0) {
                result = plus1;
            } else if (value > 0) {
                result = minus1;
            }
        }
        return result;
    }

    private static void processStream(Stream<Double[]> pairStream) {
        //какие-то операции с value и refValue
        pairStream.forEach( a -> {
            Double value = a[0];
            Double refValue = a[1];
            System.out.print(value);
            System.out.print(":");
            System.out.println(refValue);
        });
    }
}

PS Больше года не пишу на Java — сейчас Java синтаксис кажется ужас. Для простейших действий до черта строк кода, и велика вероятность ошибиться. Пипец, если из вакансий останется только Java и я не смогу найти работу на нормальных языках, придется на пенсию поход скоро уходить.

Здравствуйте, elmal, Вы писали:


E>Строчек кода больше, но зато здесь следование single responsibility principle и каждый из методов можно тестить по отдельности, если б я сделал их публичными.

Тонкий троллинг, зачет

Здравствуйте, c-smile, Вы писали:

CS>

CS>Double pv, cv;
CS>for (ListIterator<Double> it = list.listIterator(); it.hasNext(); ) {
CS>    pv = cv;
CS>    cv = it.next();
CS>    Double rv;    
CS>    if (pv != null && cv != 0) {
CS>        if (cv < 0)
CS>          rv = pv;
CS>        else if (cv > 0)
CS>          rv = cv;
CS>    }
CS>    ... //какие-то операции с value и refValue
CS>}
CS>

CS> ... //какие-то операции с value и refValue
А где само value-то? сv? Я так понял по условию refValue всегда != value. Оно или null или prev или next.

Здравствуйте, зиг, Вы писали:

зиг>што он делает — проходится по отсортированному списку из чисел (вида -100, -90, -2, 0, 3, 50) , и для каждого из чисел (кроме центрального — 0) находит "пару" — в зависимости оттого если число отрицательное или положительное — это или следующее число в списке, либо предыдущее. ноля быть не может. зачем такое надо не спрашивайте !!

Формально он и для нулевых элементов что-то делает (при value == 0, refVal == null).
Плюс ещё валится, если самый первый валуй позитивный, совершенно напрасно спрашивая при этом it.hasPrevious(), который всегда вернёт труё, ибо только что взяли же следующий.

Если не надо валиться и не надо ничего делать для текущего нуля, то можно как-то так

    Double prevVal = null;
    boolean needNext = false;

    for (ListIterator<Double> it = list.listIterator(); it.hasNext(); ) {
        Double curVal = it.next();
        if (needNext)
            process(prevVal, curVal);
        needNext = curVal < 0;

        if (curVal > 0 && prevVal != null)
            process(curVal, prevVal);
        prevVal = curVal;
    }

Правда при этом на одной итерации может понадобиться дважды звать обработку, за себя и за того парня, поэтому лучше её вынести в отдельную функцию. Ещё могут быть какие-то тонкие отличия ввиду того, что зачем-то обрабатываются объекты (ака пойнтеры на них), но сами объекты вроде немодифицируемые, пойнтера мы передадим в функцию, которая их (локальные) испортить не сможет, а порядок обработки должен сохраняться. Зато движение по списку строго одностороннее.


зиг>вопрос — как переписать этот уродский код чтоб было красивее. одобряется использование фич джавы 8 — если они тут подойдут

зиг>спасибо

Здравствуйте, StanislavK, Вы писали:

SK>А где само value-то? сv? Я так понял по условию refValue всегда != value. Оно или null или prev или next.

Как-то так:

Double pv, cv;
for (ListIterator<Double> it = list.listIterator(); it.hasNext(); ) {
    pv = cv;
    cv = it.next();
    Double refValue;    
    Double value;    
    if (pv != null && cv != 0) {
        if (cv < 0) { refValue = pv; value = cv; }
        else if (cv > 0) { refValue = cv; value = pv; }
    }
    ... //какие-то операции с value и refValue
}

TL;DR: смысл в том что есть sliding window из двух элементов. В зависимости от "стороны от нуля" один из этих элементов refValue, а второй value. И наоборот.

Здравствуйте, Blazkowicz, Вы писали:

B>

B>    private static class Triad<T> {
B>        public Triad(T ... triad) {
B>            this.triad = triad;
B>        }

B>

По сути это Zipper (который из ФП). Класс Triad надо сделать враппером вокруг итератора. И назвать его что-то типа ListIteratorZipper(ListIterator it).