HistoryRouter Push method can remove first item but doesn't update historyIndex

[zbarrier]

Hi, I'm looking at the code to get familiar with your routers. Great job and thank you for sharing.

Unless I'm missing something, I think there is a bug in the HistoryRouter Push method. In the scenario when count is greater than history max size, it removes the first item but doesn't adjust the historyIndex to account for this. Shouldn't the code decrement the history Index by 1 or -- to account for the removal of the first item?

I imagine this scenario is extremely rare and why no one has encountered an issue.

[sandreas]

Shouldn't the code decrement the history Index by 1 or -- to account for the removal of the first item?

This seems to me like you are not experiencing an issue, but just having a question why things work like they do?!

Well, the HistoryMaxSize is something that should prevent the list of viewModels getting too big (e.g. to save memory as well as traversing time). This is designed as kind of a moving window, that discards items of the past that are no longer within the windowSize/HistoryMaxSize. So here is a short illustration:

# HistMaxSize=3

Action: push HomeViewModel
# Items: HomeViewModel
# Index=0

Action: push SettingsViewModel, push UsersViewModel
# Items: HomeViewModel, SettingsViewModel, UsersViewModel
# Index=2


========== next action exceeds the HistMaxSize =======
Action: push RolesViewModel
# Items: SettingsViewModel, UsersViewModel, RolesViewModel
# Index=2

So the Index should not change when items are pushed beyond HistMaxSize, because everytime a new item is pushed upon the stack, another item is shifted from the beginning of the stack... Correct behaviour in my opinion. What do you think?

[zbarrier]

// ...
_history.Add(item);
_historyIndex = _history.Count - 1;
if (_history.Count > _historyMaxSize)
{
    _history.RemoveAt(0);
    // Need to add here or move the _historyIndex set below if 
    _historyIndex--;
}
// ...

_historyIndex will be equal to _history.Count instead of _history.Count - 1 without the change. This would produce an error.

[sandreas]

_historyIndex will be equal to _history.Count instead of _history.Count - 1 without the change. This would produce an error.

You may be right. I'll investigate this and probably write an xUnit-Test to prevent misbehaviour. Thank you for pointing this out.

[zbarrier]

No problem, great work on this library. I'm just getting into Avalonia and found my way to your lib, extremely helpful.