hpi-swa-teaching / PowerSqueak

A presentation tool for the Squeak development platform
3 stars 1 forks source link

Change extensions to contain a single PSExtension Object #59

Open LeonMatthes opened 6 years ago

LeonMatthes commented 6 years ago

Context menu and acceptsRescale should be handled through a single extension Object, instead of the current approach. This would allow users to add PowerSqueak context menus to Morphs, without adding the morphs code. This might also allow us to better solve #56

LeonBein commented 6 years ago

I would not agree on that. In my opion the different types of extensions should not be mixed up in order to allow greater flexibility. It does not seem right to allow a morph to have a custom context menu only if it accepts rescales and vice versa.

LeonMatthes commented 6 years ago

I agree with the argument, but I would still say, we should use a single extension object, which contains all the information, which features are supported by the extension, and, more importantly, this object should contain blocks to execute, if the feature is supported. In contrast to the current system, which basically checks, whether a Morph understands a function by checking, whether the Morph has a specific extension and then executes it, this would make for a cleaner API for anyone, who would want to better integrate a previously unsupported Morph into PowerSqueak. What I imagine such a procedure would look like:

Morph>>initialize

    | psExtension |
    psExtension := PSExtension new. "by default, all features are disabled"
    psExtension rescale: [ :extent | self rescale: extent ] "Only rescale, don't have a context-menu, for example"
    self setProperty: #PSExtension to: psExtension

While a bit more bloated, it would allow you, to easily adapt any Morph to work with PowerSqueak, even if you don't want to change its code, and it would not add as much to the extension interface, as we do have to share the extensions with other external tools, which may lead to conflicts down the road.

LeonBein commented 6 years ago

Yet, the ability to add a block for the extensions does not require a specific class, as the block may also simply be added as extension:

XYMorph>>initialize

   super initialize.
   self setProperty: #rescale to: [:newExtent | self rescale: newExtent]

Two more points: Remember Pheno getting into everything? We should avoid that. Also, adding extensions with one object per extension makes us much more flexible while still not overengineering stuff. Different extensions should be decoupled from each other. The only point I see is that some extensions might need more than one method. However, this is currently hypothetic and could also easily be avoided by going back to the "extension = flag" approach. (I think your current approach doesn't solve this problem either)