From 1aeaac8504876b3d954bb8d31da7b124678b7ce6 Mon Sep 17 00:00:00 2001 From: Julia Friesel Date: Sun, 14 Oct 2018 13:08:56 +0200 Subject: [PATCH 1/8] Don't do duplicate work in getLastMensesStart --- lib/cycle.js | 107 +++++++++++++++++++-------------------------- test/cycle.spec.js | 6 ++- 2 files changed, 49 insertions(+), 64 deletions(-) diff --git a/lib/cycle.js b/lib/cycle.js index 7deb02e..5393eb9 100644 --- a/lib/cycle.js +++ b/lib/cycle.js @@ -26,70 +26,49 @@ export default function config(opts) { minCyclesForPrediction = opts.minCyclesForPrediction || 3 } - function getLastMensesStart(targetDateString) { - const targetDate = LocalDate.parse(targetDateString) - const withWrappedDates = bleedingDaysSortedByDate - .filter(day => !day.bleeding.exclude) - .map(day => { - day.wrappedDate = LocalDate.parse(day.date) - return day - }) + function findLatestMensesStart(bleedingDays) { + if (!bleedingDays.length) return null - // the index of the first bleeding day before the target day - const index = withWrappedDates.findIndex(day => { - return ( - day.wrappedDate.isEqual(targetDate) || - day.wrappedDate.isBefore(targetDate) - ) - }) - - if (index < 0) { - withWrappedDates.forEach(day => delete day.wrappedDate) - return null - } - - const prevBleedingDays = withWrappedDates.slice(index) - - const lastMensesStart = prevBleedingDays.find((day, i) => { - return noBleedingDayWithinThreshold(day, prevBleedingDays.slice(i + 1)) + // assumes bleeding days are ordered latest first, and + // excluded values already removed + const lastMensesStart = bleedingDays.find((day, i) => { + return noBleedingDayWithinThreshold(day, bleedingDays.slice(i + 1)) }) function noBleedingDayWithinThreshold(day, previousBleedingDays) { - const periodThreshold = day.wrappedDate.minusDays(maxBreakInBleeding + 1) - return !previousBleedingDays.some(({ wrappedDate }) => { - return ( - wrappedDate.equals(periodThreshold) || - wrappedDate.isAfter(periodThreshold) - ) - }) + const localDate = LocalDate.parse(day.date) + const threshold = localDate.minusDays(maxBreakInBleeding + 1).toString() + return !previousBleedingDays.some(({ date }) => date >= threshold) } - withWrappedDates.forEach(day => delete day.wrappedDate) return lastMensesStart } - function getFollowingMensesStart(targetDateString) { - const targetDate = LocalDate.parse(targetDateString) - const withWrappedDates = bleedingDaysSortedByDate + function getLastMensesStartForDay(targetDateString) { + // the index of the first bleeding day before the target day + const index = bleedingDaysSortedByDate.findIndex(day => { + return day.date <= targetDateString && !day.bleeding.exclude + }) + + if (index < 0) return null + + const prevBleedingDays = bleedingDaysSortedByDate.slice(index) + return findLatestMensesStart(prevBleedingDays) + } + + function getFollowingMensesStartForDay(targetDateString) { + const followingBleedingDays = bleedingDaysSortedByDate .filter(day => !day.bleeding.exclude) - .map(day => { - day.wrappedDate = LocalDate.parse(day.date) - return day - }) - - const firstBleedingDayAfterTargetDay = withWrappedDates .reverse() - .find(day => { - return day.wrappedDate.isAfter(targetDate) - }) - withWrappedDates.forEach(day => delete day.wrappedDate) + const firstBleedingDayAfterTargetDay = followingBleedingDays + .find(day => day.date > targetDateString) return firstBleedingDayAfterTargetDay } function getCycleDayNumber(targetDateString) { - const lastMensesStart = getLastMensesStart(targetDateString) + const lastMensesStart = getLastMensesStartForDay(targetDateString) if (!lastMensesStart) return null const targetDate = LocalDate.parse(targetDateString) const lastMensesLocalDate = LocalDate.parse(lastMensesStart.date) @@ -111,7 +90,7 @@ export default function config(opts) { } function getPreviousCycle(dateString) { - const startOfCycle = getLastMensesStart(dateString) + const startOfCycle = getLastMensesStartForDay(dateString) if (!startOfCycle) return null const dateBeforeStartOfCycle = LocalDate .parse(startOfCycle.date) @@ -123,10 +102,10 @@ export default function config(opts) { function getCycleForDay(dayOrDate) { const dateString = typeof dayOrDate === 'string' ? dayOrDate : dayOrDate.date - const cycleStart = getLastMensesStart(dateString) + const cycleStart = getLastMensesStartForDay(dateString) if (!cycleStart) return null const cycleStartIndex = cycleDaysSortedByDate.indexOf(cycleStart) - const nextMensesStart = getFollowingMensesStart(dateString) + const nextMensesStart = getFollowingMensesStartForDay(dateString) if (nextMensesStart) { return cycleDaysSortedByDate.slice( cycleDaysSortedByDate.indexOf(nextMensesStart) + 1, @@ -137,16 +116,20 @@ export default function config(opts) { } } - function getAllMensesStarts(day, collectedDates) { - day = day || LocalDate.now().toString() - collectedDates = collectedDates || [] - const lastStart = getLastMensesStart(day) - if (!lastStart) { - return collectedDates - } else { - const newDay = LocalDate.parse(lastStart.date).minusDays(1).toString() - collectedDates.push(lastStart.date) - return getAllMensesStarts(newDay, collectedDates) + function getAllMensesStarts(initialBleedingDays = bleedingDaysSortedByDate) { + return recurse(initialBleedingDays.filter(d => !d.bleeding.exclude)) + + function recurse(bleedingDays, collectedDates) { + collectedDates = collectedDates || [] + const lastStart = findLatestMensesStart(bleedingDays) + if (!lastStart) { + return collectedDates + } else { + collectedDates.push(lastStart.date) + const index = bleedingDays.indexOf(lastStart) + const remainingDays = bleedingDays.slice(index + 1) + return recurse(remainingDays, collectedDates) + } } } @@ -189,8 +172,8 @@ export default function config(opts) { lastStart = lastStart.plusDays(periodDistance) const nextPredictedDates = [lastStart.toString()] for (let j = 0; j < periodStartVariation; j++) { - nextPredictedDates.push(lastStart.minusDays(j+1).toString()) - nextPredictedDates.push(lastStart.plusDays(j+1).toString()) + nextPredictedDates.push(lastStart.minusDays(j + 1).toString()) + nextPredictedDates.push(lastStart.plusDays(j + 1).toString()) } nextPredictedDates.sort() predictedMenses.push(nextPredictedDates) diff --git a/test/cycle.spec.js b/test/cycle.spec.js index e3b5111..9d16139 100644 --- a/test/cycle.spec.js +++ b/test/cycle.spec.js @@ -625,7 +625,8 @@ describe('getAllMensesStart', () => { const result = getAllMensesStarts() expect(result.length).to.eql(2) expect(result).to.eql(['2018-06-01', '2018-05-01']) - }), + }) + it('works for two cycle starts with excluded data', () => { const cycleDaysSortedByDate = [ { @@ -649,7 +650,8 @@ describe('getAllMensesStart', () => { const result = getAllMensesStarts() expect(result.length).to.eql(2) expect(result).to.eql(['2018-06-01', '2018-05-01']) - }), + }) + it('returns an empty array if no bleeding days are given', () => { const cycleDaysSortedByDate = [ {} ] From de4d39557ef081d8dd5912f925a7022ee99423e7 Mon Sep 17 00:00:00 2001 From: Julia Friesel Date: Sun, 14 Oct 2018 14:08:26 +0200 Subject: [PATCH 2/8] Add test for getAllMensesStart speed --- test/cycle.spec.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/cycle.spec.js b/test/cycle.spec.js index 9d16139..d61ee8b 100644 --- a/test/cycle.spec.js +++ b/test/cycle.spec.js @@ -1,6 +1,7 @@ import chai from 'chai' import dirtyChai from 'dirty-chai' import cycleModule from '../lib/cycle' +import { LocalDate } from 'js-joda' const expect = chai.expect chai.use(dirtyChai) @@ -663,4 +664,25 @@ describe('getAllMensesStart', () => { expect(result.length).to.eql(0) expect(result).to.eql([]) }) + + it('is not slow with 500 menses starts', () => { + const startDate = LocalDate.parse('2018-10-01') + const cycleDaysSortedByDate = Array(500) + .fill(null) + .map((_, i) => { + return { + date: startDate.minusMonths(i).toString(), + bleeding: { value: 2 } + } + }) + const { getAllMensesStarts } = cycleModule({ + cycleDaysSortedByDate, + bleedingDaysSortedByDate: cycleDaysSortedByDate + }) + const start = Date.now() + const result = getAllMensesStarts() + const duration = Date.now() - start + expect(result.length).to.eql(500) + expect(duration).to.be.lessThan(100) + }) }) \ No newline at end of file From dca286a1d6220f1b24d7b372352e96233b552365 Mon Sep 17 00:00:00 2001 From: Julia Friesel Date: Mon, 15 Oct 2018 08:18:06 +0200 Subject: [PATCH 3/8] Disable notification listeners firing on app startup --- lib/notifications.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/notifications.js b/lib/notifications.js index 74427d2..0eab38a 100644 --- a/lib/notifications.js +++ b/lib/notifications.js @@ -40,14 +40,16 @@ export default function setupNotifications(navigate) { repeatType: 'day' }) } - }) + }, false) periodReminderObservable(reminder => { Notification.cancelLocalNotifications({id: '2'}) if (reminder.enabled) setupPeriodReminder() - }) + }, false) - getBleedingDaysSortedByDate().addListener(() => { + getBleedingDaysSortedByDate().addListener((_, changes) => { + // the listener fires on setup, so we check if there were actually any changes + if (nothingChanged(changes)) return Notification.cancelLocalNotifications({id: '2'}) if (periodReminderObservable.value.enabled) setupPeriodReminder() }) @@ -77,4 +79,8 @@ function setupPeriodReminder() { }) } } +} + +function nothingChanged(dbChanges) { + return Object.values(dbChanges).every(changeArray => changeArray.length === 0) } \ No newline at end of file From 124f6cfce702dc40dfac1b16c72884cc9cbbb5ab Mon Sep 17 00:00:00 2001 From: Julia Friesel Date: Mon, 15 Oct 2018 09:34:07 +0200 Subject: [PATCH 4/8] Use nothing-changed helper in home to prevent unnecessary render --- components/home.js | 4 +++- helpers/db-unchanged.js | 3 +++ lib/notifications.js | 5 +---- 3 files changed, 7 insertions(+), 5 deletions(-) create mode 100644 helpers/db-unchanged.js diff --git a/components/home.js b/components/home.js index 56e59e8..364ba27 100644 --- a/components/home.js +++ b/components/home.js @@ -11,6 +11,7 @@ import { getOrCreateCycleDay, getCycleDaysSortedByDate } from '../db' import { getFertilityStatusForDay } from '../lib/sympto-adapter' import styles from '../styles' import AppText, { AppTextLight } from './app-text' +import nothingChanged from '../helpers/db-unchanged' export default class Home extends Component { constructor(props) { @@ -32,7 +33,8 @@ export default class Home extends Component { this.cycleDays.addListener(this.updateState) } - updateState = () => { + updateState = (_, changes) => { + if (nothingChanged(changes)) return const prediction = this.getBleedingPrediction() const fertilityStatus = getFertilityStatusForDay(this.todayDateString) this.setState({ diff --git a/helpers/db-unchanged.js b/helpers/db-unchanged.js new file mode 100644 index 0000000..4625a80 --- /dev/null +++ b/helpers/db-unchanged.js @@ -0,0 +1,3 @@ +export default function (dbChanges) { + return Object.values(dbChanges).every(changeArray => changeArray.length === 0) +} \ No newline at end of file diff --git a/lib/notifications.js b/lib/notifications.js index 0eab38a..8c1934d 100644 --- a/lib/notifications.js +++ b/lib/notifications.js @@ -5,6 +5,7 @@ import Moment from 'moment' import { settings as labels } from '../components/labels' import { getOrCreateCycleDay, getBleedingDaysSortedByDate } from '../db' import cycleModule from './cycle' +import nothingChanged from '../helpers/db-unchanged' export default function setupNotifications(navigate) { Notification.configure({ @@ -80,7 +81,3 @@ function setupPeriodReminder() { } } } - -function nothingChanged(dbChanges) { - return Object.values(dbChanges).every(changeArray => changeArray.length === 0) -} \ No newline at end of file From e465b2a81694e4ffcb1a34e4eb8c4aab2445f742 Mon Sep 17 00:00:00 2001 From: Julia Friesel Date: Mon, 15 Oct 2018 14:07:47 +0200 Subject: [PATCH 5/8] Use nothingChanged in calendar and chart, too --- components/calendar.js | 5 +++-- components/chart/chart.js | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/components/calendar.js b/components/calendar.js index 4ffce88..4134540 100644 --- a/components/calendar.js +++ b/components/calendar.js @@ -5,7 +5,7 @@ import { getOrCreateCycleDay, getBleedingDaysSortedByDate } from '../db' import cycleModule from '../lib/cycle' import {shadesOfRed} from '../styles/index' import styles from '../styles/index' - +import nothingChanged from '../helpers/db-unchanged' export default class CalendarView extends Component { constructor(props) { @@ -21,7 +21,8 @@ export default class CalendarView extends Component { this.bleedingDays.addListener(this.setStateWithCalFormattedDays) } - setStateWithCalFormattedDays = () => { + setStateWithCalFormattedDays = (_, changes) => { + if (nothingChanged(changes)) return const predictedMenses = cycleModule().getPredictedMenses() this.setState({ bleedingDaysInCalFormat: toCalFormat(this.bleedingDays), diff --git a/components/chart/chart.js b/components/chart/chart.js index 11f5142..14d03ce 100644 --- a/components/chart/chart.js +++ b/components/chart/chart.js @@ -19,6 +19,7 @@ import MucusIcon from '../../assets/mucus' import NoteIcon from '../../assets/note' import PainIcon from '../../assets/pain' import SexIcon from '../../assets/sex' +import nothingChanged from '../../helpers/db-unchanged' export default class CycleChart extends Component { constructor(props) { @@ -48,7 +49,8 @@ export default class CycleChart extends Component { if (this.state.chartHeight) return const height = nativeEvent.layout.height this.setState({ chartHeight: height }) - this.reCalculateChartInfo = () => { + this.reCalculateChartInfo = (_, changes) => { + if (nothingChanged(changes)) return // how many symptoms need to be displayed on the chart's upper symptom row? this.symptomRowSymptoms = [ 'bleeding', From 4f6a705c69b91f1004cd6cf71bc28683a7bdaa15 Mon Sep 17 00:00:00 2001 From: Julia Friesel Date: Mon, 15 Oct 2018 14:41:44 +0200 Subject: [PATCH 6/8] Remove listeners on layout before re-adding them --- components/chart/chart.js | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/components/chart/chart.js b/components/chart/chart.js index 14d03ce..87c3717 100644 --- a/components/chart/chart.js +++ b/components/chart/chart.js @@ -48,9 +48,7 @@ export default class CycleChart extends Component { onLayout = ({ nativeEvent }) => { if (this.state.chartHeight) return const height = nativeEvent.layout.height - this.setState({ chartHeight: height }) - this.reCalculateChartInfo = (_, changes) => { - if (nothingChanged(changes)) return + const reCalculateChartInfo = () => { // how many symptoms need to be displayed on the chart's upper symptom row? this.symptomRowSymptoms = [ 'bleeding', @@ -66,8 +64,8 @@ export default class CycleChart extends Component { }) }) - this.xAxisHeight = this.state.chartHeight * config.xAxisHeightPercentage - const remainingHeight = this.state.chartHeight - this.xAxisHeight + this.xAxisHeight = height * config.xAxisHeightPercentage + const remainingHeight = height - this.xAxisHeight this.symptomHeight = config.symptomHeightPercentage * remainingHeight this.symptomRowHeight = this.symptomRowSymptoms.length * this.symptomHeight @@ -78,16 +76,35 @@ export default class CycleChart extends Component { this.chartSymptoms.push('temperature') } - const columnData = this.makeColumnInfo() - this.setState({ columns: columnData }) + const columnData = this.makeColumnInfo(nfpLines(), this.chartSymptoms) + this.setState({ + columns: columnData, + chartHeight: height + }) } - this.cycleDaysSortedByDate.addListener(this.reCalculateChartInfo) - this.removeObvListener = scaleObservable(this.reCalculateChartInfo, false) + reCalculateChartInfo() + this.updateListeners(reCalculateChartInfo) + } + + updateListeners(dataUpdateHandler) { + // remove existing listeners + if(this.handleDbChange) { + this.cycleDaysSortedByDate.removeListener(this.handleDbChange) + } + if (this.removeObvListener) this.removeObvListener() + + this.handleDbChange = (_, changes) => { + if (nothingChanged(changes)) return + dataUpdateHandler() + } + + this.cycleDaysSortedByDate.addListener(this.handleDbChange) + this.removeObvListener = scaleObservable(dataUpdateHandler, false) } componentWillUnmount() { - this.cycleDaysSortedByDate.removeListener(this.reCalculateChartInfo) + this.cycleDaysSortedByDate.removeListener(this.handleDbChange) this.removeObvListener() } From ef7f6452487ec2e6d8404308fd77cd86eeffc9c8 Mon Sep 17 00:00:00 2001 From: Julia Friesel Date: Mon, 22 Oct 2018 21:20:01 +0200 Subject: [PATCH 7/8] Add explainer for db-unchanged --- helpers/db-unchanged.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/helpers/db-unchanged.js b/helpers/db-unchanged.js index 4625a80..49bd73f 100644 --- a/helpers/db-unchanged.js +++ b/helpers/db-unchanged.js @@ -1,3 +1,7 @@ +// when data changes, realm gives us an object with updates +// https://realm.io/docs/javascript/latest/#collection-notifications +// but it sometimes gets fired even though there are no changes +// - we want to check for that and see if all arrays are empty export default function (dbChanges) { return Object.values(dbChanges).every(changeArray => changeArray.length === 0) } \ No newline at end of file From 1b20658eedafaf4065bcb277c6b0bf5ceb8ccc64 Mon Sep 17 00:00:00 2001 From: Julia Friesel Date: Mon, 22 Oct 2018 21:24:54 +0200 Subject: [PATCH 8/8] Move db-unchanged to db dir --- components/calendar.js | 2 +- components/chart/chart.js | 2 +- components/home.js | 2 +- {helpers => db}/db-unchanged.js | 0 lib/notifications.js | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) rename {helpers => db}/db-unchanged.js (100%) diff --git a/components/calendar.js b/components/calendar.js index 4134540..fba53c7 100644 --- a/components/calendar.js +++ b/components/calendar.js @@ -5,7 +5,7 @@ import { getOrCreateCycleDay, getBleedingDaysSortedByDate } from '../db' import cycleModule from '../lib/cycle' import {shadesOfRed} from '../styles/index' import styles from '../styles/index' -import nothingChanged from '../helpers/db-unchanged' +import nothingChanged from '../db/db-unchanged' export default class CalendarView extends Component { constructor(props) { diff --git a/components/chart/chart.js b/components/chart/chart.js index 87c3717..b4b4453 100644 --- a/components/chart/chart.js +++ b/components/chart/chart.js @@ -19,7 +19,7 @@ import MucusIcon from '../../assets/mucus' import NoteIcon from '../../assets/note' import PainIcon from '../../assets/pain' import SexIcon from '../../assets/sex' -import nothingChanged from '../../helpers/db-unchanged' +import nothingChanged from '../../db/db-unchanged' export default class CycleChart extends Component { constructor(props) { diff --git a/components/home.js b/components/home.js index 364ba27..730d2e8 100644 --- a/components/home.js +++ b/components/home.js @@ -11,7 +11,7 @@ import { getOrCreateCycleDay, getCycleDaysSortedByDate } from '../db' import { getFertilityStatusForDay } from '../lib/sympto-adapter' import styles from '../styles' import AppText, { AppTextLight } from './app-text' -import nothingChanged from '../helpers/db-unchanged' +import nothingChanged from '../db/db-unchanged' export default class Home extends Component { constructor(props) { diff --git a/helpers/db-unchanged.js b/db/db-unchanged.js similarity index 100% rename from helpers/db-unchanged.js rename to db/db-unchanged.js diff --git a/lib/notifications.js b/lib/notifications.js index 8c1934d..717aad3 100644 --- a/lib/notifications.js +++ b/lib/notifications.js @@ -5,7 +5,7 @@ import Moment from 'moment' import { settings as labels } from '../components/labels' import { getOrCreateCycleDay, getBleedingDaysSortedByDate } from '../db' import cycleModule from './cycle' -import nothingChanged from '../helpers/db-unchanged' +import nothingChanged from '../db/db-unchanged' export default function setupNotifications(navigate) { Notification.configure({