On 31.05.2012, at 20:39, Chris Muller wrote:
Here's a question for cmm:
While looking at your MCFileRepositoryInspector code it seems to me that versionNames isn't even used anymore, but only allVersionNames. Indeed, when I remove the super send in MCFileRepositoryInspector>>initializeVersionNames and instead add #refreshEmphasis, everything appears to work fine. So, could the super send be removed for good?
Hi. One of the side-goals of the MCRepository upgrade was to unify the RepositoryInspectors to work generically with any MCRepository. This was initially started in Monticello-cmm.423 and completed by Monticello-cmm.425 with the removal of MCFileRepositoryInspector in its entirety.
But because FileBasedRepository's can only access the entire directory contents to do any kind of interrogation of the versions present, it became wasteful to do that under the new sustainable API, which only requires to know the versions of the *selected* package.
Therefore, an entirely new MCFileRepositoryInspector subclass was created in Monticello-cmm.427 with its cache of 'allVersionNames'. Now, it could execute its own #versionNamesForSelectedPackage without having to re-get all the directory entries. MCFileRepositoryInspector is unwanted, but tolerated solely for the improved performance.
So, let's please not inch our way toward more dependency on 'allVersionNames'. We may not have any senders of versionNames from a MCFileRepositoryInspector today but it is fair game to be called in the future since it is part of the standard API for the MCRepositoryInspector.
If we could remove it, then my new nil check in MCRepositoryInspector>>initializeVersionNames wouldn't be necessary anymore ...
We should not remove the super send, but I think your nil guard is appropriate in any case. Oh, BTW, Array empty is better than Array new because it uses a single canonicalized empty Array which can be shared throughout the system. I don't think Squeak's MC is portable to other Smalltalks as it is, so it should be ok to use its niceties.
Also, I assume the reason for introducing allVersionNames was to avoid asking the repository on every package list click. However, wouldn't it have been simpler to do in MCRepositoryInspector, not MCFileRepositoryInspector? Right now we have a weird mix where the versionNames variable is unused etc.
#allVersionNames is unsustainable and has no business anywhere but in FileBased[Repository|Inspector]'s. versionNames is absolutely used for inspectors of any other type of MCRepository than the FileBased.
- Chris
Makes sense. Thanks!
- Bert -