Code coverage for DateValidatorDueWorkdayMonth and EveryDueWorkdayMonth

[FMorschel]

Related to #17, #18 and #19.

As I was developing #20, I encountered a question I'd like to discuss here.

Context

I created the shared_private.dart and developed the getWorkdayNumberInMonth function inside it. This function is used inside both classes and is tested by itself as well as from inside the classes calls to it.

Main discussion topic

Inside EveryDueWorkdayMonth.next and EveryDueWorkdayMonth.previous, I call _calculate, this private method processes the received date with the same custom logic for both methods.

Implementation of _calculate

DateTime _calculate(
  DateTime date, {
  required DateTime Function(DateTime date) dateGeneratorFunction,
  required bool isNext,
}) {
  var local = date.copyWith();
  if (_workdays.invalid(date)) {
    if (isNext) {
      local = _workdays.next(local);
    } else {
      local = _workdays.previous(local);
    }
  }
  final measuredWorkday = getWorkdayNumberInMonth(local);
  if ((isNext && (measuredWorkday >= dueWorkday)) ||
      (!isNext && (measuredWorkday <= dueWorkday))) {
    local = local.copyWith(month: local.month + (isNext ? 1 : -1), day: 1);
  }
  if (isNext) {
    local = _workdays.startDate(local.firstDayOfMonth);
  } else {
    local = local.lastDayOfMonth.when(
      _workdays.valid,
      orElse: _workdays.previous,
    )!;
  }
  while (invalid(local)) {
    local = dateGeneratorFunction(local);
  }
  return local;
}

There is a specific part of the implementation of _calculate I'd like to discuss:

  //...
  if (isNext) {
    local = _workdays.startDate(local.firstDayOfMonth);
  } else {
    local = local.lastDayOfMonth.when(
      _workdays.valid,
      orElse: _workdays.previous,
    )!;
  }
  //...

This part of the implementation covers and makes sure that some calculations are not irrelevant. Since we know that we can skip some days.

Now, here is the thing, inside next and previous an anonymous function is passed as the dateGeneratorFunction parameter for _calculate.

Implementation of next

DateTime next(DateTime date) {
  return _calculate(
    valid(date) ? _workdays.next(date) : date,
    dateGeneratorFunction: (date) {
      final measuredWorkday = getWorkdayNumberInMonth(date);
      if (measuredWorkday < dueWorkday) {
        if ((dueWorkday - measuredWorkday) >= _workdaysInWeek) {
          final currentEvery = Weekday.from(date).every;
          return currentEvery.next(date);
        }
        return _workdays.next(date);
      }
      return _workdays.startDate(_monthFirst.next(date));
    },
    isNext: true,
  );
}

Implementation of previous

DateTime previous(DateTime date) {
  return _calculate(
    date,
    dateGeneratorFunction: (date) {
      final measuredWorkday = getWorkdayNumberInMonth(date);
      if (measuredWorkday > dueWorkday) {
        if ((measuredWorkday - dueWorkday) >= _workdaysInWeek) {
          final currentEvery = Weekday.from(date).every;
          return currentEvery.previous(date);
        }
        return _workdays.previous(date);
      }
      date = _monthLast.previous(date);
      if (_workdays.valid(date)) return date;
      return _workdays.previous(date);
    },
    isNext: false,
  );
}

From these implementations, I'd like to discuss the following lines:

From next:

final measuredWorkday = getWorkdayNumberInMonth(date);
if (measuredWorkday < dueWorkday) {
  //...
}
return _workdays.startDate(_monthFirst.next(date)); // This line more precisely

From previous:

if (measuredWorkday > dueWorkday) {
  //...
}
// The following three lines more precisely:
date = _monthLast.previous(date);
if (_workdays.valid(date)) return date;
return _workdays.previous(date);

Main question

When running tests and code coverage (#22) for these new classes, I found that the lines I mentioned above from next and previous methods are not being run even once. Of course that has to do with the first bit I mentioned about the _calculate function.

What should I do now;

1. Pretend that this case will never happen and simply remove from next and previous this test.
2. Remove the highlighted part from _calculate and leave it to next and previous.
3. Separate this anonymous function that is passed in both cases into named functions and test them.

In this last case, which I'm leaning more towards. Do I create a new class to put them in and test them? Maybe add getWorkdayNumberInMonth inside as well?

Should I do the same thing I did with getWorkdayNumberInMonth? I'm not completely sure about leaving the three functions unrelated to each other.

Last thing, if I do this, any ideas about how to name them?

[FMorschel]

What I ended up deciding to be my chosen method to go forward with this package to keep everything tidy is that I'll have getWorkdayNumberInMonth and all currently private methods for those classes moved up to a new helper class. It will be visible by default only inside the current package.

If I decide to make them available to the public, I can simply add another library to export there. But I think this will at least make them testable and make sure that the package users don't get confused with a bunch of extra helping methods inside an Every/DateValidator implementation.