Friday, April 4, 2014

Little rant about some runaway pharo refactorings

I'm still trying to explore different ways to play with MC/git/filetree on the pharo-vm substrat.

But there's a serious hurdle, it's named Pharo2.0 image, and unfortunately the VM is produced by this brand.

IMO, it's the worst brand ever. I much much prefer Pharo3.0.

Pharo2.0 tools are constantly getting on my way, here is a small example of a disastrous change.

I have merged several branches directly with git in a batch, I know it won't managed MC meta data, but I want  to see how it goes.
In parallel, I have merged the MC version from package-cache at image side.
When I try to save my merged package on filetree with MC, there is a wait dialog opening but not progressing.
 

Hmm let's interrupt with CMD+SHIFT+. the process is blocked in resultSemaphore wait 
MCRepositoryGroup>>includesVersionNamed: aString
    " check for existing version name in parallel over all repositories "
    | resultSemaphore results |
    resultSemaphore := Semaphore new.
    results := Array new: self repositories size.
   
    self repositories doWithIndex: [:repository :index |
        " fork of test for each repository "
        [[ results
            at: index
            put: (repository includesVersionNamed: aString)
        ] ensure: [ resultSemaphore signal ]] fork ].

    " wait for all requests to finish "
    self repositories size timesRepeat: [ resultSemaphore wait ].
    " check if any repository included the given versionName already"
    ^ results anySatisfy: [:result| result = true ]  
 


Huh? The purpose of anySatisfy: is to exit the loop at first match.
Here we will scan all the repositories, in parallel (well there is a single native thread so far, but this can eventually avoid some long network interrupts if only I/O are non blocking).
Alas, in all case, it will scan all repositories, a tremendous work defeating the purpose of anySatisfy:. Not even thinking what will happen if one is unreachable?
Though, it can make an interesting subject for a future post: how to implement a real parallel version of anySatisfy: that is to launch several processes, stop them at this first positive hit and return?

But there is worse than this unprogressive dialog, since I broke a filetree monticello.meta/version via incorrect git merge, a debugger wanted to open in background, but didn't until I manually interrupted the wait. Ah great tools.

Now let's see how it was cleaned up in Pharo 3.0:

includesVersionNamed: aString
    " check for existing version name in parallel over all repositories "
    | results |

    results := Array new: self repositories size.
   
    self repositories doWithIndex: [:repository :index |
        " fork of test for each repository "
        results
            at: index
            put: (repository includesVersionNamed: aString) ].

    " check if any repository included the given versionName already"
    ^ results anySatisfy: [:result| result = true ] 


Good to sea there is no more fork but in the comments! For sure Pharo code is better... than Pharo.
But don't you see several smells remaining?

In parallel, take a look at Squeak method:

includesVersionNamed: aString
    ^ repositories anySatisfy: [ : each | [each includesVersionNamed: aString] on: Error do: [false]]



OK, OK, Pharo has a license to break things when they are too much entangled. It's not my favourite strategy, but it can reward faster, especially when starting from Squeak.
But in this particular case I fail to see how. Bad code is bad code.

OK, OK, It's easy to pick one wrong fix and criticize.
But the problem is that 2.0 tools are so unusable that I'm quite sure that such wrong refactorings are not isolated.

Squeak is plenty of dust and has room for improvment, no doubt about that.
But it took many years to accumulate all that mud in Squeak, I would say.
If Pharo is accumulating its own at this pace, waouh, I better understand now how the number of closed issues can grow exponentially ;)

Hey, it's a joke, I don't want to ruin the moral of Stephane and Marcus, so I'll mitigate: I see plenty of positive refactorings coming with 3.0, and I wish Pharo will succeed in making a better Squeak. So please enforce the reviews!
Count this post as my contribution of the day to the review process.

No comments:

Post a Comment