Function template names forward iterator while in fact it requires a random access iterator

[kkarbowiak]

The TWRoute::replace template function specifies the template parameter needs to meet requirements for forward iterator, while in fact it requires the iterator type to implement operator<= and operator<, which is provided by random access iterator.

vroom/src/structures/vroom/tw_route.cpp

Lines 1007 to 1057 in 176f57a

	template <std::forward_iterator Iter>
	void TWRoute::replace(const Input& input,
	const Amount& delivery,
	const Iter first_job,
	const Iter last_job,
	const Index first_rank,
	const Index last_rank) {
	assert(first_job <= last_job);
	assert(first_rank <= last_rank);
	const auto& v = input.vehicles[vehicle_rank];
	PreviousInfo current(0, 0);
	NextInfo next(0, 0);
	// Value initialization differ whether there are actually jobs added
	// or not.
	if (first_job < last_job) {
	current = previous_info(input, *first_job, first_rank);
	next = next_info(input, *(last_job - 1), last_rank);
	} else {
	// This is actually a removal as no jobs are inserted.
	current.earliest = v_start;
	next.latest = v_end;
	if (first_rank > 0) {
	const auto& previous_job = input.jobs[route[first_rank - 1]];
	const auto previous_index = previous_job.index();
	current.earliest = earliest[first_rank - 1] + action_time[first_rank - 1];
	current.location_index = previous_index;
	if (last_rank < route.size()) {
	next.latest = latest[last_rank];
	next.travel =
	v.duration(previous_index, input.jobs[route[last_rank]].index());
	} else {
	if (has_end) {
	next.travel = v.duration(previous_index, v.end.value().index());
	}
	}
	} else {
	if (last_rank < route.size()) {
	next.latest = latest[last_rank];
	if (has_start) {
	current.location_index = v.start.value().index();
	next.travel = v.duration(v.start.value().index(),
	input.jobs[route[last_rank]].index());
	}
	}
	}
	}

[kkarbowiak]

The iterator type also needs to implement operator-(int).

[jcoupey]

I think the containers we use and pass iterators from to replace all meet the random access requirement. Happy to change the type here if we can be more precise, but would this not be superseded by #1090 anyway?

[kkarbowiak]

I think the containers we use and pass iterators from to replace all meet the random access requirement.

As far as I have seen, only std::vector is used and its iterators are indeed random access ones.

Happy to change the type here if we can be more precise, but would this not be superseded by #1090 anyway?

I think it would be superseded. On the other hand ranges also have their traits (like forward range, random access range), and we could constrain the ranges just like we constrain the iterators.

[jcoupey]

Done in #1104.