Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement daily worked time rounding #248

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Implement daily worked time rounding #248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/src/main/java/org/zephyrsoft/trackworktime/options/Key.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ public enum Key {

DECIMAL_TIME_SUMS("keyShowDecimalTimeAmounts", DataType.BOOLEAN, null, R.string.showDecimalTimeAmounts),

FLATTENING_ENABLED("keyFlatteningEnabled", DataType.BOOLEAN, null, R.string.flatteningEnabled),
SMALLEST_TIME_UNIT("keySmallestTimeUnit", DataType.INTEGER, FLATTENING_ENABLED, R.string.smallestTimeUnit),
ROUNDING_ENABLED("keyRoundingEnabled", DataType.BOOLEAN, null, R.string.roundingEnabled),
SMALLEST_TIME_UNIT("keySmallestTimeUnit", DataType.INTEGER, ROUNDING_ENABLED, R.string.smallestTimeUnit),

LOCATION_BASED_TRACKING_ENABLED("keyLocationBasedTrackingEnabled", DataType.BOOLEAN, null,
R.string.enableLocationBasedTracking),
Expand Down
27 changes: 24 additions & 3 deletions app/src/main/java/org/zephyrsoft/trackworktime/timer/TimeCalculatorV2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import static org.zephyrsoft.trackworktime.util.DateTimeUtil.truncateEventToMinute;
import static org.zephyrsoft.trackworktime.util.DateTimeUtil.truncateEventsToMinute;

import android.content.SharedPreferences;

import org.apache.commons.lang3.NotImplementedException;
import org.pmw.tinylog.Logger;
import org.zephyrsoft.trackworktime.database.DAO;
Expand All @@ -28,6 +30,7 @@
import org.zephyrsoft.trackworktime.model.TargetEnum;
import org.zephyrsoft.trackworktime.model.TimeInfo;
import org.zephyrsoft.trackworktime.model.TypeEnum;
import org.zephyrsoft.trackworktime.options.Key;
import org.zephyrsoft.trackworktime.util.DateTimeUtil;

import java.time.DayOfWeek;
Expand Down Expand Up @@ -114,6 +117,9 @@ public boolean containsEvents() {
private final boolean handleFlexiTime;
private final ZoneId zoneId;

private final boolean roundingEnabled;
private final int smallestTimeUnit;

private LocalDate startDate;
private Event lastEventBeforeDay;
private LocalDate currentDate;
Expand All @@ -138,17 +144,24 @@ public boolean containsEvents() {
private long currentBalance = 0;
private int futureWorkDays = -1;

public TimeCalculatorV2(DAO dao, TimerManager timerManager, LocalDate startDate, boolean handleFlexiTime) {
public TimeCalculatorV2(DAO dao, TimerManager timerManager, LocalDate startDate, SharedPreferences preferences) {
this.dao = dao;
this.timerManager = timerManager;

this.handleFlexiTime = handleFlexiTime;
this.handleFlexiTime = preferences.getBoolean(Key.ENABLE_FLEXI_TIME.getName(), false);
if (handleFlexiTime) {
this.flexiReset = timerManager.getFlexiReset();
} else {
this.flexiReset = FlexiReset.NONE;
}

this.roundingEnabled = preferences.getBoolean(Key.ROUNDING_ENABLED.getName(), false);
Copy link
Owner

@mathisdt mathisdt Aug 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this. is superfluous here (above in this.flexiReset it's also superfluous).

if (preferences.getBoolean(Key.ROUNDING_ENABLED.getName(), false)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be

if (roundingEnabled) {

instead (we just evaluated the same statement used here).

this.smallestTimeUnit = Integer.parseInt(preferences.getString(Key.SMALLEST_TIME_UNIT.getName(), "1"));
} else {
this.smallestTimeUnit = 1;
Comment on lines +161 to +162
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The else part can be left out if the variable is initialized with the value 1 above:

private final int smallestTimeUnit = 1;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I am missing something, but I get an error if I do that.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, my bad - the final keyword is the problem. With

private int smallestTimeUnit = 1;

it works.

}

this.zoneId = timerManager.getHomeTimeZone();

setStartDate(startDate);
Expand Down Expand Up @@ -361,7 +374,7 @@ public void calculateNextDay() {

currentDayHasEvents = (events != null && !events.isEmpty());

workedTime += calculateWorkTime(events);
workedTime += roundUpToMultiple(calculateWorkTime(events), smallestTimeUnit);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be wrapped in an if block to only execute it when required:

if (roundingEnabled) {
   workedTime += roundUpToMultiple(calculateWorkTime(events), smallestTimeUnit);
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While glancing over this again, my previous comment doesn't make sense. The calculateWorkTime(events) would also be only executed if my code snippet was used, so it better should be like:

if (roundingEnabled) {
	workedTime += roundUpToMultiple(calculateWorkTime(events), smallestTimeUnit);
} else {
	workedTime += calculateWorkTime(events);
}


// always show real worked time
currentDayActual = workedTime;
Expand Down Expand Up @@ -553,4 +566,12 @@ public void calculatePeriod(PeriodEnum period, boolean includeFlexiTime) {
this.currentDayActual = currentDayActual;
this.currentDayTarget = currentDayTarget;
}

public static long roundUpToMultiple(long value, long multiple) {
if (multiple == 1) {
return value;
} else {
return (value + multiple - 1) / multiple * multiple;
Copy link
Owner

@mathisdt mathisdt Aug 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work in all cases. For the inputs value = 43, multiple = 15 it should round to 45. But it doesn't, the method would return 57 with these inputs.

To be on the safe side, I'd recommend creating a unit test for the calulation method where some "normal" cases and many (if not all) of the edge cases are checked. For a start you can copy TimeCalculatorTest to TimeCalculatorV2Test and adjust the content accordingly.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible values of multiple are: 1, 2, 3, 4, 5, 6, 10, 12, 15, 20, 30, 60

But probably only the values 5, 10, 15, 30 and 60 will be used in real life...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that's weird. I'll write some tests for it.

Also, I do use 6 as a multiple myself since my work tracks time to an accuracy of 0.1 hours. Therefore I think it makes sense to leave it up to the user to choose any divisor of 60 :)

}
}
}
10 changes: 5 additions & 5 deletions app/src/main/java/org/zephyrsoft/trackworktime/timer/TimerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,13 +307,13 @@ public long calculateTimeSum(LocalDate date, PeriodEnum periodEnum) {

switch (periodEnum) {
case DAY: {
TimeCalculatorV2 timeCalc = new TimeCalculatorV2(dao, this, date, true);
TimeCalculatorV2 timeCalc = new TimeCalculatorV2(dao, this, date, preferences);
timeCalc.calculatePeriod(PeriodEnum.DAY, false);
return timeCalc.getTimeWorked();
}

case WEEK: {
TimeCalculatorV2 timeCalc = new TimeCalculatorV2(dao, this, date, true);
TimeCalculatorV2 timeCalc = new TimeCalculatorV2(dao, this, date, preferences);
timeCalc.calculatePeriod(PeriodEnum.WEEK, false);
return timeCalc.getTimeWorked();
}
Expand Down Expand Up @@ -363,13 +363,13 @@ public Integer getMinutesRemaining() {

if (!isFollowedByWorkDay(weekDay) || toZeroEveryDay) {
// reach zero today
TimeCalculatorV2 timeCalc = new TimeCalculatorV2(dao, this, dateTime.toLocalDate(), true);
TimeCalculatorV2 timeCalc = new TimeCalculatorV2(dao, this, dateTime.toLocalDate(), preferences);
timeCalc.calculatePeriod(PeriodEnum.DAY, true);

minutesRemaining += (int)-timeCalc.getBalance();
} else {
// not the last work day of the week, distribute remaining work time
TimeCalculatorV2 timeCalc = new TimeCalculatorV2(dao, this, dateTime.toLocalDate(), true);
TimeCalculatorV2 timeCalc = new TimeCalculatorV2(dao, this, dateTime.toLocalDate(), preferences);
timeCalc.calculatePeriod(PeriodEnum.WEEK, false);

long remainingWeekPerDay = -timeCalc.getBalance() / (timeCalc.getFutureWorkDays() + 1);
Expand Down Expand Up @@ -433,7 +433,7 @@ public synchronized TimeInfo getTimesAt(LocalDate targetDate) {
Logger.debug("Date range to calculate: {} -> {}", startDate, targetDate);
Logger.debug("Number of days to calculate: {}", iter);

TimeCalculatorV2 timeCalc = new TimeCalculatorV2(dao, this, startDate,true);
TimeCalculatorV2 timeCalc = new TimeCalculatorV2(dao, this, startDate, preferences);
timeCalc.setStartSums(ret);

// FIXME background task
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/java/org/zephyrsoft/trackworktime/weektimes/WeekStateCalculator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ private void loadWeek(WeekState weekState, boolean decimalAmounts) {
}

try {
TimeCalculatorV2 timeCalc = new TimeCalculatorV2(dao, timerManager, startDate, handleFlexiTime);
TimeCalculatorV2 timeCalc = new TimeCalculatorV2(dao, timerManager, startDate, preferences);
timeCalc.setStartSums(timerManager.getTimesAt(startDate));

weekState.topLeftCorner = activity.getString(R.string.weekNumber, startDate.get(IsoFields.WEEK_OF_WEEK_BASED_YEAR));
Expand Down
4 changes: 2 additions & 2 deletions app/src/main/res/values/keys.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
<string name="keyFlexiTimeDayFriday">keyFlexiTimeDayFriday</string>
<string name="keyFlexiTimeDaySaturday">keyFlexiTimeDaySaturday</string>
<string name="keyFlexiTimeDaySunday">keyFlexiTimeDaySunday</string>
<string name="keyFlatteningCategory">keyFlatteningCategory</string>
<string name="keyFlatteningEnabled">keyFlatteningEnabled</string>
<string name="keyRoundingCategory">keyRoundingCategory</string>
<string name="keyRoundingEnabled">keyRoundingEnabled</string>
<string name="keySmallestTimeUnit">keySmallestTimeUnit</string>
<string name="keyLocationBasedTrackingCategory">keyLocationBasedTrackingCategory</string>
<string name="keyLocationBasedTrackingEnabled">keyLocationBasedTrackingEnabled</string>
Expand Down
5 changes: 3 additions & 2 deletions app/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@
<string name="flexiTimeStartValue">Start Value for Flexi Time</string>
<string name="flexiTimeStartValueDescription">in hours:minutes, e.g. 7:45</string>
<string name="smallestTimeUnit">Smallest Time Unit</string>
<string name="smallestTimeUnitDescription">in minutes, e.g. 15 - flattening rounds to this amount of time (has to be a divisor of 60)</string>
<string name="smallestTimeUnitDescription">in minutes, e.g. 15 (has to be a divisor of 60) - when time rounding is enabled, the worked time per day is rounded up to the closest multiple of this</string>
<string name="timeRounding">Time Rounding</string>
<string name="backward">jumped backward</string>
<string name="forward">jumped forward</string>
<string name="weeks">weeks</string>
Expand Down Expand Up @@ -105,7 +106,7 @@
<string name="autoPauseBeginDescription">in 24 hour time, e.g. 12.30</string>
<string name="autoPauseEnd">End of Automatic Pause</string>
<string name="autoPauseEndDescription">in 24 hour time, e.g. 13.30</string>
<string name="flatteningEnabled">Enable Time Flattening</string>
<string name="roundingEnabled">Enable Time Rounding</string>
<string name="type">Type</string>
<string name="date">Date</string>
<string name="time">Time</string>
Expand Down
10 changes: 4 additions & 6 deletions app/src/main/res/xml/options.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,20 +70,18 @@
android:summary="@string/flexiTimeDayDescription"
android:title="@string/sundayLong" />
</PreferenceCategory>
<!--
<PreferenceCategory
android:key="@string/keyFlatteningCategory"
android:title="@string/timeFlattening" >
android:key="@string/keyRoundingCategory"
android:title="@string/timeRounding" >
<CheckBoxPreference
android:key="@string/keyFlatteningEnabled"
android:title="@string/flatteningEnabled" />
android:key="@string/keyRoundingEnabled"
android:title="@string/roundingEnabled" />

<EditTextPreference
android:key="@string/keySmallestTimeUnit"
android:summary="@string/smallestTimeUnitDescription"
android:title="@string/smallestTimeUnit" />
</PreferenceCategory>
-->

<PreferenceCategory
android:key="@string/keyUserInterfaceCategory"
Expand Down