Review Board

1.0

Some adjustments to ETHistoryManager API

Updated 1 year, 6 months ago

Truls Becken Reviewers
EtoileCore
None Etoile trunk (etoile/trunk/Etoile)
Günther gave some feedback on my history manager implementation a month or so ago.

First, he found it confusing that next/previous did not return objects, but only changed the state. I can see how this is not entirely intuitive. To fix that, I want to add back/forward for state changing only, and make next/previous return objects. The only problem I see with this is that next/previous now become polymorphic because MKMusicPlayer has void methods using those names.

Günther also asked what happens to the existing history when changing the maximum history size, and what is the advantage of having a max size when the history being modeled is infinite.

I had not really thought that anybody would call setMaxHistorySize: after adding objects, but this could very well be a requirement if an application lets the user change the history size in a preferences panel. So let's complete that part of the code.

About the advantage of having a max size; it's just to save some memory if you don't care about browsing all the way back to Big Bang. I changed the default to not having a max size, following the principle of least surprise. Should there be a constructor variant that sets a custom maximum?

 
Ship it!
Posted 1 year, 7 months ago (January 12th, 2009, 3:14 p.m.)
David suggested renaming the methods that return objects to -nextObject and -previousObject. I think this is a good idea because it avoids confusion with the MK methods and is similar to -currentObject.

I wanted to update the diff, but it gives me errors. Possibly because of the Base directory, which was . when uploading the original diff.

In the update, I also wanted to make the -init method a little more robust by doing;

    ASSIGN(history, [[NSMutableArray alloc] init]);
    DESTROY(future);
Ship it!
Posted 1 year, 7 months ago (January 12th, 2009, 3:18 p.m.)
And I did NOT tick the Ship it box. That's bug number two in as many minutes.