TimeWindow::length should be scaled

[kkarbowiak]

The length member of TimeWindow class defined here:

vroom/src/structures/vroom/time_window.h

Line 21 in f5f078f

Duration length;

is initialised here:

vroom/src/structures/vroom/time_window.cpp

Lines 22 to 25 in f5f078f

	TimeWindow::TimeWindow(UserDuration start, UserDuration end)
	: start(utils::scale_from_user_duration(start)),
	end(utils::scale_from_user_duration(end)),
	length(end - start) {

Due to both start and end members being shadowed by start and end arguments, the computed length value is based on UserDuration instead of Duration.

At the moment it is not a big deal, because TimeWindow::length is used only in vehicle comparison:

vroom/src/structures/vroom/vehicle.h

Lines 138 to 153 in f5f078f

	friend bool operator<(const Vehicle& lhs, const Vehicle& rhs) {
	// Sort by:
	// - decreasing max_tasks
	// - decreasing capacity
	// - decreasing TW length
	// - decreasing range (max travel time and distance)
	return std::tie(rhs.max_tasks,
	rhs.capacity,
	rhs.tw.length,
	rhs.max_travel_time,
	rhs.max_distance) < std::tie(lhs.max_tasks,
	lhs.capacity,
	lhs.tw.length,
	lhs.max_travel_time,
	lhs.max_distance);
	}

, which works correctly whether or not the value is scaled, and in job:

vroom/src/structures/vroom/job.cpp

Lines 17 to 22 in f5f078f

	inline Duration get_tw_length(const std::vector<TimeWindow>& tws) {
	return std::accumulate(std::next(tws.begin()),
	tws.end(),
	tws[0].length,
	[](auto sum, auto tw) { return sum + tw.length; });
	}

, where it is used to initialise Job::tw_length, which itself is not used anywhere and gets removed in #1014.

Anyway, to avoid any possible confusion in the future, I would propose to either scale the value: length(utils::scale_from_user_duration(end - start)) or add a length() member function and compute the value on the fly.

[jcoupey]

Good catch. Indeed not a problem for the sorting we currently do but we should definitely get consistent values in case we use the length in conjunction with something else in the future. Can you add that to #1014?

[jcoupey]

Landed with #1014.