[squeak-dev] The Trunk: Tools-mt.1074.mczMarcel Taeumel marcel.taeumel at hpi.deWed Dec 1 09:31:14 UTC 2021
Hi all -- Those are very interesting perspectives! I have to think about this a little bit more. There is definitely something we can and should do in the long term. Best, Marcel Am 30.11.2021 20:38:08 schrieb Eliot Miranda <eliot.miranda at gmail.com>: Hi Christoph, Hi Marcel, Hi Ron, Hi All, On Mon, Nov 29, 2021 at 12:38 PM Thiede, Christoph <Christoph.Thiede at student.hpi.uni-potsdam.de [mailto:Christoph.Thiede at student.hpi.uni-potsdam.de]> wrote: Hi Marcel, > We might want to have this for all kinds of models. Sorry, but I'm not convinced. :-) The pattern is cool and very flexible, but as a downside, boilerplate code increases while readability and code exploration become harder - for instance, we do not have the proper tooling to find all "assignments" to a property or to enumerate all properties in a class. And implementing this tooling does not look straightforward to me because programmers could easily use a complex expression (self setProperty: self fileDirectorySelector toValue: ...) rather than a literal symbol for a property key so that a simple string/AST search would be insufficient - the hurdle for programmers is lower in this case than for instance variables, where they would be required to explicitly use the metaprogramming construct #instVarNamed:. Also, it becomes even harder to achieve encapsulation of an object's state. I very much agree with Kent Beck's arguments about this pattern: > Warning! This is not a pattern you should use often, if ever. It is here because you will see code written with it, and you will have to understand what is going on > The problem with code like this is that you can’t read it and understand what is going on. The programming tools don’t support this style of programming well. There is no equivalent of “browse instance variable references” for code that stores all its state in Dictionaries. It is certainly legitimate to build models where state is sometimes present and sometimes not, even in instances of the same class. You wouldn’t want to declare a hundred variables almost all of which are nil in almost all instances, just because a few instances will need them. Twe thing is, he's only half right. One *can* have support in the tools. In fact, in Tweak, part of Andreas Raab's Croquet kernel, CObject and TObject both define myProperties to hold instance state added in sumclasses. Consequently we have added support to the tools for finding references to state via messages. Now this raises an interesting point. Right now there is somewhat of a confusion between policy and mechanism in the searching the tools do. If we resolve this confusion I think we can improve our tools significantly. Let me give an example. Right now we search for inst var reads and writes by using wrappers around CompiledCode's readsField: and writesField: methods respectively. This is appropriate where inst vars are implemented as normal Smalltalk nst vars, but it means that when one searches for inst var references the tools *don't* - reveal mentions of a given inst var in a class comment - reveal mentions of an inst var in class side methods that might introspect using the inst var name If SystemNavigation were to separate the mechanism (CompiledCode>>readsField:/writesField:) from the policy ("show me all references to this inst var within this class hierarchy; and I don't just mean references in code, I also mean references in class and method comments, and anywhere else you can think to look"), then we could do a better job of helping a programmer find all effective references, while still preserving the useful concrete semantics of methods such as readsField: & writesField:, and we could also easily accomodate useful patterns such as "state in property dictionaries". Indeed a class could provide class-sode metadata to inform a tool such as SystemNavigation of the representational choices it makes, etc. To allow the tools to display the results of results that intermix references in code with references in text we need meta handle objects that allow MethodReference (the standard meta handle object for a method) to live alongside, say, ClassCommentReference, and SharedPoolReference, etc. Is this compelling or not? So I think models that really need it should implement a variable state themselves, but we should not explicitly promote this (sometimes anti-)pattern. Hypothetically, provide a trait (TVariableState?) for it, maybe. I'm not even sure whether it was a good idea to introduce this on Morph. As a general solution for "complex optional state", I would myself prefer classical composition via a nullable variable for a MyOptionalState instance. > Why do you see this as a "crazy hack"? It's rather interesting to have a tool that is able to reflect on its internals the same way as it does on external (user specified) information. ;-) Feels like a the textual version of a halo for a command line (or text buffer). I see that point for self-inspection and I am not against such a hook in general. But code evaluated in a workspace should just work like outside a workspace (with the only exception of automatic variables), so to address the name collision issue, you could at least use a prefix that is not allowed in Smalltalk variable names ($#, $ , $:, whatever). Still, IMO one variable should serve one purpose, which is why I consider this a crazy hack - the rule of separation of concerns would tell me to use a separate dictionary for properties than for bindings. The only legitimate reason to reuse the first dict that I could think of would be storage optimization - but I don't think we need this in this case (and if yes, lazy initialization of the separate dict might be sufficient, too). Coming back to the self-inspection "feature": I don't actually see where you would need this - in particular, given the very limited set of properties that are currently available -, but if we want to pursue this idea, from a user perspective, I would consider something like "thisWorkspace fileDirectory", "thisWorkspace containingWindow", etc. less surprising, but more clear and powerful. Still, we should discuss whether it is a good idea to risk possible naming conflicts by this. In this example, maybe "thisWorkspace" should be a default entry in the normal bindings directory rather than a special syntax feature. > > I only need to print "_fileDirectory" in a workspace and the menu will break. > That's a bug. Why should a read operation change the bindings dictionary? I did not know that. Should be an easy fix. :-) I have relied on this behavior a few times to see what parts of a script I have evaluated already. But that's arguably not a legitimate use case, so thank you for the fix! :-) Best, Christoph Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org [mailto:squeak-dev-bounces at lists.squeakfoundation.org]> im Auftrag von Taeumel, Marcel Gesendet: Montag, 29. November 2021 17:41:44 An: squeak-dev Betreff: Re: [squeak-dev] The Trunk: Tools-mt.1074.mcz Hi Christoph -- Given that I needed to mimic the flexibility of morph's properties for the sake of robustness, we can add an extra "properties" instVar if necessary. Actually adding instVars for a non-essential extra in the Workspace tool felt overkill. We might want to have this for all kinds of models. Would make extensions easier. So, it's not a "new thing" per se, but an idea present in morphs applied to models. Why do you see this as a "crazy hack"? It's rather interesting to have a tool that is able to reflect on its internals the same way as it does on external (user specified) information. ;-) Feels like a the textual version of a halo for a command line (or text buffer). If you think that the risk of a name collision in bindings is too high, we can choose a different prefix or add an extra dictionary instVar. But then we would loose that cool self-reflection feature. :-O > I only need to print "_fileDirectory" in a workspace and the menu will break. That's a bug. Why should a read operation change the bindings dictionary? I did not know that. Should be an easy fix. :-) Best, Marcel Am 27.11.2021 21:46:44 schrieb Thiede, Christoph <christoph.thiede at student.hpi.uni-potsdam.de [mailto:christoph.thiede at student.hpi.uni-potsdam.de]>: Hi Marcel, crazy hack, but it bites with the fact that storing underscore variables in a Workspace is a legal operation. I only need to print "_fileDirectory" in a workspace and the menu will break. Could we please just use a separate dictionary for the properties? :-) Furthermore, how many different properties can actually be stored in a Workspace? So far I only counted 3 different keys - does this really justify the use of the Variable State pattern? Best, Christoph Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org [mailto:squeak-dev-bounces at lists.squeakfoundation.org]> im Auftrag von commits at source.squeak.org [mailto:commits at source.squeak.org] <commits at source.squeak.org [mailto:commits at source.squeak.org]> Gesendet: Donnerstag, 25. November 2021 11:44:04 An: squeak-dev at lists.squeakfoundation.org [mailto:squeak-dev at lists.squeakfoundation.org]; packages at lists.squeakfoundation.org [mailto:packages at lists.squeakfoundation.org] Betreff: [squeak-dev] The Trunk: Tools-mt.1074.mcz Marcel Taeumel uploaded a new version of Tools to project The Trunk: http://source.squeak.org/trunk/Tools-mt.1074.mcz [http://source.squeak.org/trunk/Tools-mt.1074.mcz] ==================== Summary ==================== Name: Tools-mt.1074 Author: mt Time: 25 November 2021, 11:44:01.944354 am UUID: 39954290-b416-f242-a5b0-c7aea0e3c6a6 Ancestors: Tools-mt.1073 Adds a property protocol to Workspace to avoid having to go through #containingWindow to store extra stuff, which does not even work for MVC, just for morphs (i.e., SystemWindow, not StandardSystemView). Reuses a workspace's "bindings" with each property being prefixed with a "_" to not interfere with regular bindings. Note that such extra properties have already been added to #containingWindow in the past. =============== Diff against Tools-mt.1073 =============== Item was added: + ----- Method: Workspace>>hasProperty: (in category 'binding - properties') ----- + hasProperty: aSymbol + + | propertyValue | + propertyValue := self valueOfProperty: aSymbol. + propertyValue ifNil: [^ false]. + propertyValue == false ifTrue: [^ false]. + ^ true! Item was changed: ----- Method: Workspace>>initialize (in category 'initialize-release') ----- initialize super initialize. + bindings := Dictionary new. - self initializeBindings. acceptDroppedMorphs := false. mustDeclareVariables := self class declareVariablesAutomatically not. environment := Environment current! Item was removed: - ----- Method: Workspace>>initializeBindings (in category 'binding') ----- - initializeBindings - - bindings := Dictionary new! Item was added: + ----- Method: Workspace>>removeProperty: (in category 'binding - properties') ----- + removeProperty: aSymbol + + bindings removeKey: ('_', aSymbol) asSymbol ifAbsent: [].! Item was added: + ----- Method: Workspace>>resetBindings (in category 'binding') ----- + resetBindings + "Remove all bindings that are not prefixed with an $_. See #setProperty:toValue:." + + bindings keysAndValuesRemove: [:key :value | key first ~= $_]! Item was added: + ----- Method: Workspace>>setProperty:toValue: (in category 'binding - properties') ----- + setProperty: aSymbol toValue: anObject + + anObject ifNil: [^ self removeProperty: aSymbol]. + bindings at: ('_', aSymbol) asSymbol put: anObject.! Item was added: + ----- Method: Workspace>>valueOfProperty: (in category 'binding - properties') ----- + valueOfProperty: aSymbol + + ^ self valueOfProperty: aSymbol ifAbsent: nil! Item was added: + ----- Method: Workspace>>valueOfProperty:ifAbsent: (in category 'binding - properties') ----- + valueOfProperty: aSymbol ifAbsent: aBlock + + ^ bindings at: ('_', aSymbol) asSymbol ifAbsent: aBlock! -- _,,,^..^,,,_ best, Eliot -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20211201/9d9743a7/attachment.html>
More information about the Squeak-dev mailing list |