From e87927f57b27db0899003a1bb932ed596b2d400c Mon Sep 17 00:00:00 2001 From: Tore Langedal Endestad Date: Mon, 28 Mar 2022 13:22:01 +0200 Subject: [PATCH] =?UTF-8?q?Enda=20raskere=20combine,=20unng=C3=A5r=20bruk?= =?UTF-8?q?=20av=20deprecated=20kode=20(#83)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Enda raskere combine, unngår bruk av deprecated kode * Endringer for å gjøre SonarCloud fornøyd * Ytterligere forbedringer - quick exit ikke lenger nødvendig, fjernes fordi den også kan gi ClassCastException for DISJOINT ved bruk av Combinator --- .../fpsak/tidsserie/LocalDateTimeline.java | 227 +++++++++--------- .../LocalDateTimelineYtelseTest.java | 10 +- 2 files changed, 111 insertions(+), 126 deletions(-) diff --git a/src/main/java/no/nav/fpsak/tidsserie/LocalDateTimeline.java b/src/main/java/no/nav/fpsak/tidsserie/LocalDateTimeline.java index fc80188..5283093 100644 --- a/src/main/java/no/nav/fpsak/tidsserie/LocalDateTimeline.java +++ b/src/main/java/no/nav/fpsak/tidsserie/LocalDateTimeline.java @@ -9,14 +9,12 @@ import java.util.Collections; import java.util.HashSet; import java.util.Iterator; -import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import java.util.NavigableMap; import java.util.NavigableSet; +import java.util.NoSuchElementException; import java.util.Objects; import java.util.Optional; -import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; import java.util.concurrent.atomic.AtomicReference; @@ -204,24 +202,18 @@ public LocalDateTimeline combine(final LocalDateSegment other, *

* beholder inntil videre for å kunne teste ny mot gammel implementasjon */ - @Deprecated LocalDateTimeline combineGammel(final LocalDateTimeline other, final LocalDateSegmentCombinator combinator, final JoinStyle combinationStyle) { - LocalDateTimeline quickExit = combinationStyle.checkQuickExit(this, other); - if (quickExit != null) { - return quickExit; - } - // Join alle intervaller - final NavigableMap joinDatoInterval = joinLocalDateIntervals(getDatoIntervaller(), - other.getDatoIntervaller()); + final NavigableMap joinDatoInterval = joinLocalDateIntervals(getLocalDateIntervals(), + other.getLocalDateIntervals()); // filtrer ut i henhold til combinationStyle final List> combinedSegmenter = new ArrayList<>(); final LocalDateTimeline myTidslinje = this; joinDatoInterval.entrySet().stream() - .filter(e -> combinationStyle.accept(e.getValue())) + .filter(e -> combinationStyle.accept((e.getValue() & LHS) > 0, (e.getValue() & RHS) > 0)) .forEachOrdered(e -> { LocalDateInterval key = e.getKey(); LocalDateSegment nyVerdi = combinator.combine(key, myTidslinje.getSegment(key), @@ -238,29 +230,46 @@ LocalDateTimeline combineGammel(final LocalDateTimeline other, fina * Kombinerer to tidslinjer, med angitt combinator funksjon og {@link JoinStyle}. */ public LocalDateTimeline combine(final LocalDateTimeline other, final LocalDateSegmentCombinator combinator, final JoinStyle combinationStyle) { - - LocalDateTimeline quickExit = combinationStyle.checkQuickExit(this, other); - if (quickExit != null) { - return quickExit; + List> combinedSegmenter = new ArrayList<>(); + Iterator> lhsIterator = this.segments.iterator(); + Iterator> rhsIterator = other.segments.iterator(); + LocalDateSegment lhs = lhsIterator.hasNext() ? lhsIterator.next() : null; + LocalDateSegment rhs = rhsIterator.hasNext() ? rhsIterator.next() : null; + Iterator startdatoIterator = new KnekkpunktIterator<>(this.segments, other.segments); + if (!startdatoIterator.hasNext()) { + return empty(); //begge input-tidslinjer var tomme } - NavigableSet intervallerIKombinertTidslinje = utedIntervallerIKombinertTidslinje(toSegments(), other.toSegments(), combinationStyle); - - //henter alle verdier med en gang, slipper å iterere i hver tidslinje en gang pr intervall - Map> aktuelleVerdierFraDenneTidslinje = this.getSegments(intervallerIKombinertTidslinje); - Map> aktuelleVerdierFraAndreTidslinje = other.getSegments(intervallerIKombinertTidslinje); + LocalDate fom = startdatoIterator.next(); + while (startdatoIterator.hasNext()) { + lhs = spolTil(lhs, lhsIterator, fom); + rhs = spolTil(rhs, rhsIterator, fom); - List> combinedSegmenter = new ArrayList<>(); - intervallerIKombinertTidslinje.stream().forEachOrdered(key -> { - LocalDateSegment nyVerdi = combinator.combine(key, aktuelleVerdierFraDenneTidslinje.get(key), aktuelleVerdierFraAndreTidslinje.get(key)); - if (nyVerdi != null) { - combinedSegmenter.add(nyVerdi); + boolean harLhs = lhs != null && lhs.getLocalDateInterval().contains(fom); + boolean harRhs = rhs != null && rhs.getLocalDateInterval().contains(fom); + LocalDate nesteFom = startdatoIterator.next(); + if (combinationStyle.accept(harLhs, harRhs)) { + LocalDateInterval periode = new LocalDateInterval(fom, nesteFom.minusDays(1)); + LocalDateSegment tilpassetLhsSegment = harLhs ? splittVedDelvisOverlapp(this.segmentSplitter, lhs, periode) : null; + LocalDateSegment tilpassetRhsSegment = harRhs ? splittVedDelvisOverlapp(other.segmentSplitter, rhs, periode) : null; + LocalDateSegment nyVerdi = combinator.combine(periode, tilpassetLhsSegment, tilpassetRhsSegment); + if (nyVerdi != null) { + combinedSegmenter.add(nyVerdi); + } } - }); - + fom = nesteFom; + } return new LocalDateTimeline<>(combinedSegmenter); } + private static LocalDateSegment splittVedDelvisOverlapp(SegmentSplitter segmentSplitter, LocalDateSegment segment, LocalDateInterval ønsketIntervall) { + if (segment == null || segment.getLocalDateInterval().equals(ønsketIntervall)) { + return segment; + } + return segmentSplitter.apply(ønsketIntervall, segment); + } + + /** * Fikser opp tidslinjen slik at tilgrensende intervaller med equal verdi får et sammenhengende intervall. Nyttig * for å 'redusere' tidslinjen ned til enkleste form før lagring etc. @@ -410,31 +419,6 @@ public LocalDateSegment getSegment(LocalDateInterval datoInterval) { } } - private Map> getSegments(NavigableSet datoInterval) { - if (isEmpty() || datoInterval.isEmpty()) { - return Map.of(); - } - - Iterator> segmentIterator = segments.iterator(); - LocalDateSegment segment = segmentIterator.next(); - - Map> resultat = new LinkedHashMap<>(); - for (LocalDateInterval datoIntervall : datoInterval) { - while (segment != null && segment.getTom().isBefore(datoIntervall.getFomDato())) { - segment = segmentIterator.hasNext() ? segmentIterator.next() : null; - } - if (segment != null && segment.getLocalDateInterval().overlaps(datoIntervall)) { - if (segment.getLocalDateInterval().equals(datoIntervall)) { - resultat.put(datoIntervall, segment); - } else { - resultat.put(datoIntervall, segmentSplitter.apply(datoIntervall, segment)); - } - } - } - return resultat; - } - - @Override public int hashCode() { return segments.hashCode(); @@ -610,7 +594,7 @@ public String toString() { (isEmpty() ? "0" //$NON-NLS-1$ : getMinLocalDate() + ", " + getMaxLocalDate()) //$NON-NLS-1$ + " [" + size() + "]" //$NON-NLS-1$ // $NON-NLS-2$ - + "> = [" + getDatoIntervaller().stream().map(String::valueOf).collect(Collectors.joining(",")) + "]" //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + + "> = [" + getLocalDateIntervals().stream().map(String::valueOf).collect(Collectors.joining(",")) + "]" //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ ; } @@ -796,53 +780,74 @@ private NavigableMap joinLocalDateIntervals(Navigabl return joined; } - private NavigableSet utedIntervallerIKombinertTidslinje(NavigableSet> lhsIntervaller, NavigableSet> rhsIntervaller, JoinStyle combinationStyle) { - if (lhsIntervaller.isEmpty()) { - return combinationStyle.accept(RHS) ? toLocalDateIntervals(rhsIntervaller) : new TreeSet<>(); - } - if (rhsIntervaller.isEmpty()) { - return combinationStyle.accept(LHS) ? toLocalDateIntervals(lhsIntervaller) : new TreeSet<>(); + /** + * Finner alle knekkpunkter fra to tidslinjer, i sekvens. + *

+ * Knekkpunkter er 'start av et intervall' og 'dagen etter slutt av et intervall'. Sistnevnte fordi det da kan være starten på et nytt intervall + *

+ * Tidliger implementert ved å dytte alle knekkpunkter i et TreeSet og iterere, men dette er raskere O(n) vs O(n ln n), og bruker mindre minne + */ + private static class KnekkpunktIterator implements Iterator { + private final Iterator> lhsIterator; + private final Iterator> rhsIterator; + private LocalDateSegment lhsSegment; + private LocalDateSegment rhsSegment; + private LocalDate next; + + public KnekkpunktIterator(NavigableSet> lhsIntervaller, NavigableSet> rhsIntervaller) { + lhsIterator = lhsIntervaller.iterator(); + rhsIterator = rhsIntervaller.iterator(); + lhsSegment = lhsIterator.hasNext() ? lhsIterator.next() : null; + rhsSegment = rhsIterator.hasNext() ? rhsIterator.next() : null; + + next = lhsSegment != null ? lhsSegment.getFom() : null; + if (rhsSegment != null && (next == null || rhsSegment.getFom().isBefore(next))) { + next = rhsSegment.getFom(); + } } - NavigableSet joined = new TreeSet<>(); - Iterator> lhsIterator = lhsIntervaller.iterator(); - Iterator> rhsIterator = rhsIntervaller.iterator(); - LocalDateSegment lhs = lhsIterator.next(); - LocalDateSegment rhs = rhsIterator.next(); - Set startdatoKandidater = finKnekkpunkter(lhsIntervaller, rhsIntervaller); - Iterator startdatoIterator = startdatoKandidater.iterator(); - LocalDate fom = startdatoIterator.next(); - while (startdatoIterator.hasNext()) { - lhs = spolTil(lhs, lhsIterator, fom); - rhs = spolTil(rhs, rhsIterator, fom); - boolean lhsMatch = lhs != null && lhs.getLocalDateInterval().contains(fom); - boolean rhsMatch = rhs != null && rhs.getLocalDateInterval().contains(fom); - int combinedFlags = (lhsMatch ? LHS : 0) | (rhsMatch ? RHS : 0); - LocalDate nesteFom = startdatoIterator.next(); - if (combinedFlags > 0 && combinationStyle.accept(combinedFlags)) { - joined.add(new LocalDateInterval(fom, nesteFom.minusDays(1))); + @Override + public LocalDate next() { + if (next == null) { + throw new NoSuchElementException("Ikke flere verdier igjen"); } - fom = nesteFom; + LocalDate denne = next; + oppdaterNeste(); + return denne; } - return joined; - } + @Override + public boolean hasNext() { + return next != null; + } - private static NavigableSet toLocalDateIntervals(NavigableSet> rhsIntervaller) { - return rhsIntervaller.stream().map(LocalDateSegment::getLocalDateInterval).collect(Collectors.toCollection(TreeSet::new)); - } + private void oppdaterNeste() { + while (lhsSegment != null && !lhsSegment.getTom().plusDays(1).isAfter(next)) { + lhsSegment = lhsIterator.hasNext() ? lhsIterator.next() : null; + } + while (rhsSegment != null && !rhsSegment.getTom().plusDays(1).isAfter(next)) { + rhsSegment = rhsIterator.hasNext() ? rhsIterator.next() : null; + } - private static Set finKnekkpunkter(NavigableSet> lhsIntervaller, NavigableSet> rhsIntervaller) { - Set startdatoKandidater = new TreeSet<>(); - for (LocalDateSegment intervall : lhsIntervaller) { - startdatoKandidater.add(intervall.getFom()); - startdatoKandidater.add(intervall.getTom().plusDays(1)); + LocalDate forrige = next; + //neste knekkpunkt kan komme fra hvilken som helst av de to tidsseriene, må sjekke begge + next = oppdaterKandidatForNeste(forrige, null, lhsSegment); + next = oppdaterKandidatForNeste(forrige, next, rhsSegment); } - for (LocalDateSegment intervall : rhsIntervaller) { - startdatoKandidater.add(intervall.getFom()); - startdatoKandidater.add(intervall.getTom().plusDays(1)); + + private static LocalDate oppdaterKandidatForNeste(LocalDate forrige, LocalDate besteKandidat, LocalDateSegment segment) { + if (segment != null) { + LocalDate fomKandidat = segment.getFom(); + if (fomKandidat.isAfter(forrige) && (besteKandidat == null || fomKandidat.isBefore(besteKandidat))) { + return fomKandidat; + } + LocalDate tomKandidat = segment.getTom().plusDays(1); + if (tomKandidat.isAfter(forrige) && (besteKandidat == null || tomKandidat.isBefore(besteKandidat))) { + return tomKandidat; + } + } + return besteKandidat; } - return startdatoKandidater; } private static LocalDateSegment spolTil(LocalDateSegment intervall, Iterator> iterator, LocalDate fom) { @@ -873,8 +878,8 @@ public enum JoinStyle { */ CROSS_JOIN { @Override - public boolean accept(int option) { - return option > 0; + public boolean accept(boolean harLhs, boolean harRhs) { + return harLhs || harRhs; } }, /** @@ -882,14 +887,8 @@ public boolean accept(int option) { */ DISJOINT { @Override - public boolean accept(int option) { - return option == LHS; - } - - @SuppressWarnings("unchecked") - @Override - protected LocalDateTimeline checkQuickExit(LocalDateTimeline lhs, LocalDateTimeline rhs) { - return rhs.isEmpty() || lhs.isEmpty() ? (LocalDateTimeline) lhs : null; + public boolean accept(boolean harLhs, boolean harRhs) { + return harLhs && !harRhs; } }, /** @@ -897,16 +896,10 @@ protected LocalDateTimeline checkQuickExit(LocalDateTimeline lhs */ INNER_JOIN { @Override - public boolean accept(int option) { - return (option & LHS) == LHS && (option & RHS) == RHS; + public boolean accept(boolean harLhs, boolean harRhs) { + return harLhs && harRhs; } - @Override - protected LocalDateTimeline checkQuickExit(LocalDateTimeline lhs, LocalDateTimeline rhs) { - boolean skip = ((lhs.isEmpty() || rhs.isEmpty()) || !new LocalDateInterval(lhs.getMinLocalDate(), lhs.getMaxLocalDate()) - .overlaps(new LocalDateInterval(rhs.getMinLocalDate(), rhs.getMaxLocalDate()))); - return skip ? new LocalDateTimeline(Collections.emptyList()) : null; - } }, /** * alltid venstre tidsserie (LHS), høyre (RHS) kun med verdi dersom matcher. Combinator funksjon må hensyn ta @@ -914,8 +907,8 @@ protected LocalDateTimeline checkQuickExit(LocalDateTimeline lhs */ LEFT_JOIN { @Override - public boolean accept(int option) { - return (option & LHS) == LHS; + public boolean accept(boolean harLhs, boolean harRhs) { + return harLhs; } }, @@ -925,17 +918,13 @@ public boolean accept(int option) { */ RIGHT_JOIN { @Override - public boolean accept(int option) { - return (option & RHS) == RHS; + public boolean accept(boolean harLhs, boolean harRhs) { + return harRhs; } }; - public abstract boolean accept(int option); + public abstract boolean accept(boolean harLhs, boolean harRhs); - @SuppressWarnings("unused") - protected LocalDateTimeline checkQuickExit(LocalDateTimeline lhs, LocalDateTimeline rhs) { - return null; - } } /** diff --git a/src/test/java/no/nav/fpsak/tidsserie/LocalDateTimelineYtelseTest.java b/src/test/java/no/nav/fpsak/tidsserie/LocalDateTimelineYtelseTest.java index 2160fa2..826f17e 100644 --- a/src/test/java/no/nav/fpsak/tidsserie/LocalDateTimelineYtelseTest.java +++ b/src/test/java/no/nav/fpsak/tidsserie/LocalDateTimelineYtelseTest.java @@ -12,14 +12,12 @@ import no.nav.fpsak.tidsserie.LocalDateTimeline.JoinStyle; @Disabled //kan kjøres lokalt for å få inntrykk av ytelse ved endringer på implementasjon -public class LocalDateTimelineYtelseTest { +class LocalDateTimelineYtelseTest { private final LocalDate today = LocalDate.now(); @Test - public void lange_tidslinjer() throws Exception { - // bruker tall til å referer relative dager til today - + void lange_tidslinjer() { for (int i = 0; i < 3000; i++) { int antall = 1000; int antallDagerPrIntervall = 5; @@ -37,9 +35,7 @@ public void lange_tidslinjer() throws Exception { } @Test - public void korte_tidslinjer() throws Exception { - // bruker tall til å referer relative dager til today - + void korte_tidslinjer() { for (int i = 0; i < 200000; i++) { int antall = 10;