I've seen a lot of progress for the Pharo Kernel by Pavel recently and I really appreciate his work. But I have some questions I think we should discuss. There have been quite a few changes to make code work in the Kernel and to reduce dependencies between packages. But I really have the impression that many of these changes are of bad quality :/. I do not want to offend people, but this is fairly important if we want to have a nice code base. Example: ======== UnixResolver >> nbGetEnv: str #NativeBoost asClassIfPresent: [ ^ self privNbGetEnv: str ]. ^ nil The goal of this code is to conditionally do something in the main image but continue to work in the kernel. Observations: ============= - #asClassIfPresent: and Co are bad - Why do we clutter the main image if the kernel should change? - Can't we solve these things by having proper other objects? - Is the UnixResolver really needed in the Kernel? I worked with Esteban last week on a similar case in the CommandLineHandler and we ended up introducing a minimal superclass. I guess this goes much in the direction of having proper NullObjects. I really think we should not add these little nasty changes, but rather introduce proper new classes that can be unloaded if needed. What do you think?
well... I said the same when I was applying those changes, he :) I already made a note in my TODO list to refactor that as soon as possible too :) Esteban On May 25, 2013, at 12:55 PM, Camillo Bruni <camillobruni@gmail.com> wrote:
I've seen a lot of progress for the Pharo Kernel by Pavel recently and I really appreciate his work. But I have some questions I think we should discuss.
There have been quite a few changes to make code work in the Kernel and to reduce dependencies between packages. But I really have the impression that many of these changes are of bad quality :/.
I do not want to offend people, but this is fairly important if we want to have a nice code base.
Example: ======== UnixResolver >> nbGetEnv: str #NativeBoost asClassIfPresent: [ ^ self privNbGetEnv: str ]. ^ nil
The goal of this code is to conditionally do something in the main image but continue to work in the kernel.
Observations: ============= - #asClassIfPresent: and Co are bad - Why do we clutter the main image if the kernel should change? - Can't we solve these things by having proper other objects? - Is the UnixResolver really needed in the Kernel?
I worked with Esteban last week on a similar case in the CommandLineHandler and we ended up introducing a minimal superclass. I guess this goes much in the direction of having proper NullObjects.
I really think we should not add these little nasty changes, but rather introduce proper new classes that can be unloaded if needed.
What do you think?
On 5/25/13 8:02 AM, "Esteban Lorenzano" <estebanlm@gmail.com> wrote:
well... I said the same when I was applying those changes, he :) I already made a note in my TODO list to refactor that as soon as possible too :)
Esteban
On May 25, 2013, at 12:55 PM, Camillo Bruni <camillobruni@gmail.com> wrote:
I've seen a lot of progress for the Pharo Kernel by Pavel recently and I really appreciate his work. But I have some questions I think we should discuss.
There have been quite a few changes to make code work in the Kernel and to reduce dependencies between packages. But I really have the impression that many of these changes are of bad quality :/.
I do not want to offend people, but this is fairly important if we want to have a nice code base.
Example: ======== UnixResolver >> nbGetEnv: str #NativeBoost asClassIfPresent: [ ^ self privNbGetEnv: str ]. ^ nil
The goal of this code is to conditionally do something in the main image but continue to work in the kernel.
Observations: ============= - #asClassIfPresent: and Co are bad - Why do we clutter the main image if the kernel should change? - Can't we solve these things by having proper other objects? - Is the UnixResolver really needed in the Kernel?
I worked with Esteban last week on a similar case in the CommandLineHandler and we ended up introducing a minimal superclass. I guess this goes much in the direction of having proper NullObjects.
I really think we should not add these little nasty changes, but rather introduce proper new classes that can be unloaded if needed.
What do you think?
Could say the url for dowloading the last PharoKernel ? Edgar
On 2013-05-26, at 10:57, "Edgar J. De Cleene" <edgardec2005@gmail.com> wrote:
On 5/25/13 8:02 AM, "Esteban Lorenzano" <estebanlm@gmail.com> wrote:
well... I said the same when I was applying those changes, he :) I already made a note in my TODO list to refactor that as soon as possible too :)
Esteban
On May 25, 2013, at 12:55 PM, Camillo Bruni <camillobruni@gmail.com> wrote:
I've seen a lot of progress for the Pharo Kernel by Pavel recently and I really appreciate his work. But I have some questions I think we should discuss.
There have been quite a few changes to make code work in the Kernel and to reduce dependencies between packages. But I really have the impression that many of these changes are of bad quality :/.
I do not want to offend people, but this is fairly important if we want to have a nice code base.
Example: ======== UnixResolver >> nbGetEnv: str #NativeBoost asClassIfPresent: [ ^ self privNbGetEnv: str ]. ^ nil
The goal of this code is to conditionally do something in the main image but continue to work in the kernel.
Observations: ============= - #asClassIfPresent: and Co are bad - Why do we clutter the main image if the kernel should change? - Can't we solve these things by having proper other objects? - Is the UnixResolver really needed in the Kernel?
I worked with Esteban last week on a similar case in the CommandLineHandler and we ended up introducing a minimal superclass. I guess this goes much in the direction of having proper NullObjects.
I really think we should not add these little nasty changes, but rather introduce proper new classes that can be unloaded if needed.
What do you think?
Could say the url for dowloading the last PharoKernel ?
You'll find it on the pharo-contribution CI: https://ci.inria.fr/pharo-contribution/view/Pharo-Kernel-3.0/
On 5/26/13 6:03 AM, "Camillo Bruni" <camillobruni@gmail.com> wrote:
You'll find it on the pharo-contribution CI: https://ci.inria.fr/pharo-contribution/view/Pharo-Kernel-3.0/
Very thanks ! Edgar
On 25 May 2013 12:55, Camillo Bruni <camillobruni@gmail.com> wrote:
I've seen a lot of progress for the Pharo Kernel by Pavel recently and I really appreciate his work. But I have some questions I think we should discuss.
There have been quite a few changes to make code work in the Kernel and to reduce dependencies between packages. But I really have the impression that many of these changes are of bad quality :/.
I do not want to offend people, but this is fairly important if we want to have a nice code base.
Example: ======== UnixResolver >> nbGetEnv: str #NativeBoost asClassIfPresent: [ ^ self privNbGetEnv: str ]. ^ nil
The goal of this code is to conditionally do something in the main image but continue to work in the kernel.
Observations: ============= - #asClassIfPresent: and Co are bad - Why do we clutter the main image if the kernel should change? - Can't we solve these things by having proper other objects? - Is the UnixResolver really needed in the Kernel?
I worked with Esteban last week on a similar case in the CommandLineHandler and we ended up introducing a minimal superclass. I guess this goes much in the direction of having proper NullObjects.
I really think we should not add these little nasty changes, but rather introduce proper new classes that can be unloaded if needed.
What do you think?
hmm.. what about factoring out this functionality in separate package, say "Kernel-Extras" and keep dependencies there. Another approach is using delegates, e.g. when kernel class provides an API while concrete implementation is provided in separate package. That i think same idea as with superclass, which defines common/required interface, and then self-configuring itself depending on what packages are in system and what services registered by them. Then, of course, you need to provide either fallback code or stub code (null pattern). -- Best regards, Igor Stasenko.
On 25 May 2013 12:40, Igor Stasenko <siguctua@gmail.com> wrote:
On 25 May 2013 12:55, Camillo Bruni <camillobruni@gmail.com> wrote:
I've seen a lot of progress for the Pharo Kernel by Pavel recently and I really appreciate his work. But I have some questions I think we should discuss.
There have been quite a few changes to make code work in the Kernel and to reduce dependencies between packages. But I really have the impression that many of these changes are of bad quality :/.
I do not want to offend people, but this is fairly important if we want to have a nice code base.
Example: ======== UnixResolver >> nbGetEnv: str #NativeBoost asClassIfPresent: [ ^ self privNbGetEnv: str ]. ^ nil
The goal of this code is to conditionally do something in the main image but continue to work in the kernel.
Observations: ============= - #asClassIfPresent: and Co are bad - Why do we clutter the main image if the kernel should change? - Can't we solve these things by having proper other objects? - Is the UnixResolver really needed in the Kernel?
I worked with Esteban last week on a similar case in the CommandLineHandler and we ended up introducing a minimal superclass. I guess this goes much in the direction of having proper NullObjects.
I really think we should not add these little nasty changes, but rather introduce proper new classes that can be unloaded if needed.
What do you think?
hmm.. what about factoring out this functionality in separate package, say "Kernel-Extras" and keep dependencies there. Another approach is using delegates, e.g. when kernel class provides an API while concrete implementation is provided in separate package. That i think same idea as with superclass, which defines common/required interface, and then self-configuring itself depending on what packages are in system and what services registered by them. Then, of course, you need to provide either fallback code or stub code (null pattern).
So if I'd read the entire thread before posting (who on earth does _that_?), I'd have read this. Yes, this is exactly what my mail meant. The low level packages can refer to _something_, and some higher level package can be plugged into _something_'s hole. frank
-- Best regards, Igor Stasenko.
On May 25, 2013, at 12:55 PM, Camillo Bruni <camillobruni@gmail.com> wrote:
I've seen a lot of progress for the Pharo Kernel by Pavel recently and I really appreciate his work. But I have some questions I think we should discuss.
There have been quite a few changes to make code work in the Kernel and to reduce dependencies between packages. But I really have the impression that many of these changes are of bad quality :/.
I do not want to offend people, but this is fairly important if we want to have a nice code base.
Example: ======== UnixResolver >> nbGetEnv: str #NativeBoost asClassIfPresent: [ ^ self privNbGetEnv: str ]. ^ nil
The goal of this code is to conditionally do something in the main image but continue to work in the kernel.
Observations: ============= - #asClassIfPresent: and Co are bad
Yes I do not like this pattern that because if just hide the problem
- Why do we clutter the main image if the kernel should change? - Can't we solve these things by having proper other objects? - Is the UnixResolver really needed in the Kernel?
I worked with Esteban last week on a similar case in the CommandLineHandler and we ended up introducing a minimal superclass. I guess this goes much in the direction of having proper NullObjects.
I really think we should not add these little nasty changes, but rather introduce proper new classes that can be unloaded if needed.
What do you think?
On 25 May 2013 11:55, Camillo Bruni <camillobruni@gmail.com> wrote:
I've seen a lot of progress for the Pharo Kernel by Pavel recently and I really appreciate his work. But I have some questions I think we should discuss.
There have been quite a few changes to make code work in the Kernel and to reduce dependencies between packages. But I really have the impression that many of these changes are of bad quality :/.
I do not want to offend people, but this is fairly important if we want to have a nice code base.
Example: ======== UnixResolver >> nbGetEnv: str #NativeBoost asClassIfPresent: [ ^ self privNbGetEnv: str ]. ^ nil
The goal of this code is to conditionally do something in the main image but continue to work in the kernel.
Observations: ============= - #asClassIfPresent: and Co are bad - Why do we clutter the main image if the kernel should change? - Can't we solve these things by having proper other objects? - Is the UnixResolver really needed in the Kernel?
I worked with Esteban last week on a similar case in the CommandLineHandler and we ended up introducing a minimal superclass. I guess this goes much in the direction of having proper NullObjects.
I really think we should not add these little nasty changes, but rather introduce proper new classes that can be unloaded if needed.
What do you think?
Like Stef says, using #asClassIfPresent makes things worse because the dependency is still there, only harder to find. Returning nil adds another problem, in that you can end up with an UndefinedObject >> #doesNotUnderstand: far away from the cause of the problem. The first way I've worked with these is moving UnixResolver >> #nbGetNev: to the NativeBoost package. That means that NativeBoost -> UnixResolver, which at least is a dependency pointing in the right direction. (I'm ignoring the fact that in this case there's clearly a platform difference issue: UnixResolver makes no sense on a Windows machine, obviously.) Another approach is to have something like an AppRegistry subclass. This is a class that has a #register: method which lets you set the class of something. Maybe it's a mail client, maybe it's the main browser class. That provides in the lower packages a hook. Other packages can just refer to the hook, and the higher level packages can use that hook to get called, without lower level packages depending on those higher level packages. frank
On Sat, May 25, 2013 at 12:55 PM, Camillo Bruni <camillobruni@gmail.com>wrote:
I've seen a lot of progress for the Pharo Kernel by Pavel recently and I really appreciate his work. But I have some questions I think we should discuss.
There have been quite a few changes to make code work in the Kernel and to reduce dependencies between packages. But I really have the impression that many of these changes are of bad quality :/.
I do not want to offend people, but this is fairly important if we want to have a nice code base.
Example: ======== UnixResolver >> nbGetEnv: str #NativeBoost asClassIfPresent: [ ^ self privNbGetEnv: str ]. ^ nil
The goal of this code is to conditionally do something in the main image but continue to work in the kernel.
Observations: ============= - #asClassIfPresent: and Co are bad - Why do we clutter the main image if the kernel should change? - Can't we solve these things by having proper other objects? - Is the UnixResolver really needed in the Kernel?
I worked with Esteban last week on a similar case in the CommandLineHandler and we ended up introducing a minimal superclass. I guess this goes much in the direction of having proper NullObjects.
I really think we should not add these little nasty changes, but rather introduce proper new classes that can be unloaded if needed.
What do you think?
Hi Cami, firstly I have to say that most of all patches I do to make Pharo Kernel jobs working are really ugly hacks of poor quality :-) Of course I'm firstly trying to make it work somehow. I do not like solutions like asClassIfPresent: too and I do not use them if I have better simple option. And if I use them, I know that I only move the problem because in most cases it crates new unimplemented calls. But that ugly hacks are only one part of the problem and maybe the minor one. The major one is that we integrate code that requires such ugly hacks :-) Nativeboost, UserManager and others are not old additions and they decrease modularity of the system. So when I find time to work on Pharo Kernel, in most cases I do not clean old problems but I need to solve the new ones. It is not the fault of authors of this additions but of our integration system. Classical eggs and chickens problem, we cannot base our integration process on Pharo Kernel processes because it is not ready and it is not ready because we do not have the integration system based on in. So I'm very glad that Esteban started to work hard on it and if we will do sometimes an ugly hack like this, we know that we do not really solve the problem but we move it to the later time. However it is more important for us to solve it soon. With more hands and attention we will be able to do it in a nicer way ;-) Cheers, -- Pavel
Hi pavel I agree with you. I think that at the end this is a problem of architecture and hooks been made to be customised. Stef
participants (7)
-
Camillo Bruni -
Edgar J. De Cleene -
Esteban Lorenzano -
Frank Shearar -
Igor Stasenko -
Pavel Krivanek -
stephane ducasse