Hi,
I created a new class Author which handles author name and initials, similar to how Pharo does, but with a better API (of course :)). To test it in your updated Trunk image just evaluate the following:
Installer squeak project: 'inbox'; addPackage: 'System-ul.495'; addPackage: 'System-ul.496'; addPackage: 'Tests-ul.166'; install.
Feedback is appreciated.
Cheers, Levente
On Fri, Sep 14, 2012 at 9:04 AM, Levente Uzonyi leves@elte.hu wrote:
I created a new class Author which handles author name and initials, similar to how Pharo does, but with a better API (of course :)).
[snip]
Feedback is appreciated.
I definitely like this refactoring. Reifying Author as a distinct object makes a lot of sense, and opens the way to adding other metadata, like email address. I also like the preservation of initials, rather than moving to full names the way Pharo does.
I do have a few questions and nits, though:
1. Why all the "PerSe" methods? Is that for compatibility with older versions of Squeak or Pharo? This seems like an indication that there's a missing concept somewhere that should be extracted to a single place.
2. I don't the idea of using #current only and forbidding #new. To temporarily switch to another author it makes more sense to swap out the full object, rather than modifying the current Author in place. Also we may want to use Author in other places—attaching them to SqueakMap accounts, or CompiledMethods, or Monticello definitions—so being able to represent authors other than the current one seems important.
3. Why #username instead of just #name?
But generally, bravo!
Colin
On Fri, 14 Sep 2012, Colin Putney wrote:
On Fri, Sep 14, 2012 at 9:04 AM, Levente Uzonyi leves@elte.hu wrote:
I created a new class Author which handles author name and initials, similar to how Pharo does, but with a better API (of course :)).
[snip]
Feedback is appreciated.
I definitely like this refactoring. Reifying Author as a distinct object makes a lot of sense, and opens the way to adding other metadata, like email address. I also like the preservation of initials, rather than moving to full names the way Pharo does.
I do have a few questions and nits, though:
- Why all the "PerSe" methods? Is that for compatibility with older
versions of Squeak or Pharo? This seems like an indication that there's a missing concept somewhere that should be extracted to a single place.
The way these methods behave is similar to how Utilities works. The *PerSe methods are just simple accessors and the accessor looking methods ensure that they are initialized.
What about removing the *PerSe methods from the instance side and update boths sides so that only the class side accessor looking methods request the initialization of the data? E.g.:
Author initials. "this will request the initials if they are not set" Author current initials. "just returns the initials" Author initialsPerSe. "just returns the initials" Author current intialsPerSe "MNU"
This would make the behavior of the instance and the class side different, which might be surprising.
Another idea is to not try to make the new implementation resemble the old one, just ensure the compatibility via the methods in Utilities.
- I don't the idea of using #current only and forbidding #new. To
temporarily switch to another author it makes more sense to swap out the full object, rather than modifying the current Author in place. Also we may want to use Author in other places—attaching them to SqueakMap accounts, or CompiledMethods, or Monticello definitions—so being able to represent authors other than the current one seems important.
I didn't want to expose the object itself, just the data, because it's fully compatible with the current API. But yes, passing the object around has benefits.
- Why #username instead of just #name?
You can write "Author username", but you can't write "Author name", because #name is used in the class system and it should return the name of the class, which is #Author. I used #name on the instance side first, but I decided to unify the two sides. If we decide to drop the class side convenience methods, then we can use #name.
But generally, bravo!
Thanks for the feedback.
Levente
Colin
On Fri, Sep 14, 2012 at 10:36 AM, Levente Uzonyi leves@elte.hu wrote:
Another idea is to not try to make the new implementation resemble the old one, just ensure the compatibility via the methods in Utilities.
My feeling is that "Author current" should be the only place where we lazily initialize by asking the user. Then Author>>initials would just be a normal accessor, and Author class>>initials would go through #current. The tricky bit with that idea is that we'd need a slightly more elaborate UI, so let the user fill in both initials and username.
One idea would be to get rid of #username for now, and have Author objects with just initials. Then in 4.5 we we'd flesh things out a bit more.
I used #name on the instance side first, but I decided to unify the two sides. If we decide to drop the class side convenience methods, then we can use #name.
Ah, that makes sense. But how about #fullName? For me, #username implies some kind of account somewhere, either the "current user" from the OS, SqueakMap, SqueakSource or the like.
Colin
Hi guys! Levente, thanks for pushing this forward!
My feeling is that "Author current" should be the only place where we lazily initialize by asking the user. Then Author>>initials would just be a normal accessor, and Author class>>initials would go through #current. The tricky bit with that idea is that we'd need a slightly more elaborate UI, so let the user fill in both initials and username.
One idea would be to get rid of #username for now, and have Author objects with just initials. Then in 4.5 we we'd flesh things out a bit more.
The way these methods behave is similar to how Utilities works. The *PerSe methods are just simple accessors and the accessor looking methods ensure that they are initialized.
What about removing the *PerSe methods from the instance side and update boths sides so that only the class side accessor looking methods request the initialization of the data? E.g.:
Author initials. "this will request the initials if they are not set" Author current initials. "just returns the initials" Author initialsPerSe. "just returns the initials" Author current intialsPerSe "MNU"
This would make the behavior of the instance and the class side different, which might be surprising.
Another idea is to not try to make the new implementation resemble the old one, just ensure the compatibility via the methods in Utilities.
Say, would y'all mind if we think of Author as a *pure domain object* and NOT couple any Squeak-specific UI behaviors to it whatsoever? We all know doing that is bad practice so how about letting Utilities take responsibility for ensuring a there is a well-formed "current Author".
That frees Author to just "be" a simple and regular kind object of which there can be multiple instances. Its just that the "current" one is the one which the rest of the system recognizes as the one authoring (saving code, etc).
Real people's names don't usually change, so let's not treat "Author current" like a global variable and temporarily maul someone's name just to accomodate some test process. Besides, then we'd have to worry about Mutexes and multi-process updating, etc..
I submitted an update to the inbox that takes this tact. With this, applications that are ok to be integrated with Squeak UI elements can use:
Utilities authorInitials
and it will make sure the Author current initials is set. Meanwhile, applications that just want to access the current value and possibly set it in their own way can simply check
Author current initials
and take action in their own way if they want.
What do you think?
On 9/14/12, Chris Muller asqueaker@gmail.com wrote:
Hi guys! Levente, thanks for pushing this forward!
My feeling is that "Author current" should be the only place where we lazily initialize by asking the user. Then Author>>initials would just be a normal accessor, and Author class>>initials would go through #current. The tricky bit with that idea is that we'd need a slightly more elaborate UI, so let the user fill in both initials and username.
One idea would be to get rid of #username for now, and have Author objects with just initials. Then in 4.5 we we'd flesh things out a bit more.
The way these methods behave is similar to how Utilities works. The *PerSe methods are just simple accessors and the accessor looking methods ensure that they are initialized.
What about removing the *PerSe methods from the instance side and update boths sides so that only the class side accessor looking methods request the initialization of the data? E.g.:
Author initials. "this will request the initials if they are not set" Author current initials. "just returns the initials" Author initialsPerSe. "just returns the initials" Author current intialsPerSe "MNU"
This would make the behavior of the instance and the class side different, which might be surprising.
Another idea is to not try to make the new implementation resemble the old one, just ensure the compatibility via the methods in Utilities.
Say, would y'all mind if we think of Author as a *pure domain object* and NOT couple any Squeak-specific UI behaviors to it whatsoever? We all know doing that is bad practice so how about letting Utilities take responsibility for ensuring a there is a well-formed "current Author".
That frees Author to just "be" a simple and regular kind object of which there can be multiple instances. Its just that the "current" one is the one which the rest of the system recognizes as the one authoring (saving code, etc).
+ 1
Real people's names don't usually change, so let's not treat "Author current" like a global variable and temporarily maul someone's name just to accomodate some test process. Besides, then we'd have to worry about Mutexes and multi-process updating, etc..
I submitted an update to the inbox that takes this tact. With this, applications that are ok to be integrated with Squeak UI elements can use:
Utilities authorInitials
and it will make sure the Author current initials is set. Meanwhile, applications that just want to access the current value and possibly set it in their own way can simply check
Author current initials
and take action in their own way if they want.
What do you think?
On Fri, 14 Sep 2012, Chris Muller wrote:
Hi guys! Levente, thanks for pushing this forward!
My feeling is that "Author current" should be the only place where we lazily initialize by asking the user. Then Author>>initials would just be a normal accessor, and Author class>>initials would go through #current. The tricky bit with that idea is that we'd need a slightly more elaborate UI, so let the user fill in both initials and username.
One idea would be to get rid of #username for now, and have Author objects with just initials. Then in 4.5 we we'd flesh things out a bit more.
The way these methods behave is similar to how Utilities works. The *PerSe methods are just simple accessors and the accessor looking methods ensure that they are initialized.
What about removing the *PerSe methods from the instance side and update boths sides so that only the class side accessor looking methods request the initialization of the data? E.g.:
Author initials. "this will request the initials if they are not set" Author current initials. "just returns the initials" Author initialsPerSe. "just returns the initials" Author current intialsPerSe "MNU"
This would make the behavior of the instance and the class side different, which might be surprising.
Another idea is to not try to make the new implementation resemble the old one, just ensure the compatibility via the methods in Utilities.
Say, would y'all mind if we think of Author as a *pure domain object* and NOT couple any Squeak-specific UI behaviors to it whatsoever? We all know doing that is bad practice so how about letting Utilities take responsibility for ensuring a there is a well-formed "current Author".
That frees Author to just "be" a simple and regular kind object of which there can be multiple instances. Its just that the "current" one is the one which the rest of the system recognizes as the one authoring (saving code, etc).
Real people's names don't usually change, so let's not treat "Author current" like a global variable and temporarily maul someone's name just to accomodate some test process. Besides, then we'd have to worry about Mutexes and multi-process updating, etc..
I submitted an update to the inbox that takes this tact. With this, applications that are ok to be integrated with Squeak UI elements can use:
Utilities authorInitials
and it will make sure the Author current initials is set. Meanwhile, applications that just want to access the current value and possibly set it in their own way can simply check
Author current initials
and take action in their own way if they want.
What do you think?
I don't like the idea to keep methods about author in Utilities. My plan is/was to rewrite all users in the image when the Author implementation is there and deprecate all author related methods in Utilities. So IMHO the class side of Author is a much better place for those methods.
Levente
On Fri, Sep 14, 2012 at 8:41 PM, Levente Uzonyi leves@elte.hu wrote:
I don't like the idea to keep methods about author in Utilities. My plan is/was to rewrite all users in the image when the Author implementation is there and deprecate all author related methods in Utilities. So IMHO the class side of Author is a much better place for those methods.
Agreed. Long-term it would be really nice to get rid of Utilities entirely.
Colin
Your version left just as many methods about author in Utilities as mine. We're not addressing that right now because we're moving forward gently and incrementally.
My main objection is popping Squeak UI-elements from directly within the domain. If you don't like Utilities to be the bridge between the Squeak UI and the domain, we can address that next. But that is independent of having Author be a pure domain.
There's also the notion of having multiple instances instead of the global-variable approach -- are you ok with that?
On Fri, Sep 14, 2012 at 10:41 PM, Levente Uzonyi leves@elte.hu wrote:
On Fri, 14 Sep 2012, Chris Muller wrote:
Hi guys! Levente, thanks for pushing this forward!
My feeling is that "Author current" should be the only place where we lazily initialize by asking the user. Then Author>>initials would just be a normal accessor, and Author class>>initials would go through #current. The tricky bit with that idea is that we'd need a slightly more elaborate UI, so let the user fill in both initials and username.
One idea would be to get rid of #username for now, and have Author objects with just initials. Then in 4.5 we we'd flesh things out a bit more.
The way these methods behave is similar to how Utilities works. The *PerSe methods are just simple accessors and the accessor looking methods ensure that they are initialized.
What about removing the *PerSe methods from the instance side and update boths sides so that only the class side accessor looking methods request the initialization of the data? E.g.:
Author initials. "this will request the initials if they are not set" Author current initials. "just returns the initials" Author initialsPerSe. "just returns the initials" Author current intialsPerSe "MNU"
This would make the behavior of the instance and the class side different, which might be surprising.
Another idea is to not try to make the new implementation resemble the old one, just ensure the compatibility via the methods in Utilities.
Say, would y'all mind if we think of Author as a *pure domain object* and NOT couple any Squeak-specific UI behaviors to it whatsoever? We all know doing that is bad practice so how about letting Utilities take responsibility for ensuring a there is a well-formed "current Author".
That frees Author to just "be" a simple and regular kind object of which there can be multiple instances. Its just that the "current" one is the one which the rest of the system recognizes as the one authoring (saving code, etc).
Real people's names don't usually change, so let's not treat "Author current" like a global variable and temporarily maul someone's name just to accomodate some test process. Besides, then we'd have to worry about Mutexes and multi-process updating, etc..
I submitted an update to the inbox that takes this tact. With this, applications that are ok to be integrated with Squeak UI elements can use:
Utilities authorInitials
and it will make sure the Author current initials is set. Meanwhile, applications that just want to access the current value and possibly set it in their own way can simply check
Author current initials
and take action in their own way if they want.
What do you think?
I don't like the idea to keep methods about author in Utilities. My plan is/was to rewrite all users in the image when the Author implementation is there and deprecate all author related methods in Utilities. So IMHO the class side of Author is a much better place for those methods.
Levente
On Sat, 15 Sep 2012, Chris Muller wrote:
Your version left just as many methods about author in Utilities as mine. We're not addressing that right now because we're moving forward gently and incrementally.
If you check those methods, all logic was removed from them, they just perform a send to Author. Btw there's a reason why I uploaded 2 packages. They have to be loaded one after the other. The first creates the Author and initializes the instance with the data from Utilities, the second updates the methods in Utilities, so the system will indirectly use Author. Loading this stuff in a single step might have unexpected side effects (e.g. loss of author information).
I didn't remove the methods from Utilities because I want to deprecate them first, but before they are deprecated, the senders from the Trunk packages have to be rewritten. I didn't want to rewrite a bunch of methods while the API is not stable, because I'd have to keep rewriting those methods while the API is changing. And imagine what would have happened if I push 10+ packages to the Inbox, noone would have taken the time to review them thoroughly, even these three seems to be too much.
My main objection is popping Squeak UI-elements from directly within the domain. If you don't like Utilities to be the bridge between the Squeak UI and the domain, we can address that next. But that is independent of having Author be a pure domain.
I think the instance side is the domain, the class side has nothing to do with it.
Utilities is an "enemy" of modularity, because it's a bunch of unrelated methods. I guess it started out as a class to hold utility methods. The problems started when they got senders from the image and the situation got worse when state was added to it.
Every user of Utilities will depend on it's package and Utilities depend on everything it uses (a bunch of packages), so if you use only one thing from it (e.g. author information) then your package will depend on a bunch of other stuff totally unrelated to your package.
There's also the notion of having multiple instances instead of the global-variable approach -- are you ok with that?
My original idea was to simply break out the author related stuff from Utilities into a single class. Since in the current system there's only a single author information maintained it was a straightforward idea to make Author a singleton. Colin proposed that multiple instances might be useful, though I still don't see any real world use cases for it, but I'm not against not overriding #new.
Levente
On Fri, Sep 14, 2012 at 10:41 PM, Levente Uzonyi leves@elte.hu wrote:
On Fri, 14 Sep 2012, Chris Muller wrote:
Hi guys! Levente, thanks for pushing this forward!
My feeling is that "Author current" should be the only place where we lazily initialize by asking the user. Then Author>>initials would just be a normal accessor, and Author class>>initials would go through #current. The tricky bit with that idea is that we'd need a slightly more elaborate UI, so let the user fill in both initials and username.
One idea would be to get rid of #username for now, and have Author objects with just initials. Then in 4.5 we we'd flesh things out a bit more.
The way these methods behave is similar to how Utilities works. The *PerSe methods are just simple accessors and the accessor looking methods ensure that they are initialized.
What about removing the *PerSe methods from the instance side and update boths sides so that only the class side accessor looking methods request the initialization of the data? E.g.:
Author initials. "this will request the initials if they are not set" Author current initials. "just returns the initials" Author initialsPerSe. "just returns the initials" Author current intialsPerSe "MNU"
This would make the behavior of the instance and the class side different, which might be surprising.
Another idea is to not try to make the new implementation resemble the old one, just ensure the compatibility via the methods in Utilities.
Say, would y'all mind if we think of Author as a *pure domain object* and NOT couple any Squeak-specific UI behaviors to it whatsoever? We all know doing that is bad practice so how about letting Utilities take responsibility for ensuring a there is a well-formed "current Author".
That frees Author to just "be" a simple and regular kind object of which there can be multiple instances. Its just that the "current" one is the one which the rest of the system recognizes as the one authoring (saving code, etc).
Real people's names don't usually change, so let's not treat "Author current" like a global variable and temporarily maul someone's name just to accomodate some test process. Besides, then we'd have to worry about Mutexes and multi-process updating, etc..
I submitted an update to the inbox that takes this tact. With this, applications that are ok to be integrated with Squeak UI elements can use:
Utilities authorInitials
and it will make sure the Author current initials is set. Meanwhile, applications that just want to access the current value and possibly set it in their own way can simply check
Author current initials
and take action in their own way if they want.
What do you think?
I don't like the idea to keep methods about author in Utilities. My plan is/was to rewrite all users in the image when the Author implementation is there and deprecate all author related methods in Utilities. So IMHO the class side of Author is a much better place for those methods.
Levente
On 16 September 2012 02:19, Levente Uzonyi leves@elte.hu wrote:
On Sat, 15 Sep 2012, Chris Muller wrote:
Your version left just as many methods about author in Utilities as mine. We're not addressing that right now because we're moving forward gently and incrementally.
If you check those methods, all logic was removed from them, they just perform a send to Author. Btw there's a reason why I uploaded 2 packages. They have to be loaded one after the other. The first creates the Author and initializes the instance with the data from Utilities, the second updates the methods in Utilities, so the system will indirectly use Author. Loading this stuff in a single step might have unexpected side effects (e.g. loss of author information).
I didn't remove the methods from Utilities because I want to deprecate them first, but before they are deprecated, the senders from the Trunk packages have to be rewritten. I didn't want to rewrite a bunch of methods while the API is not stable, because I'd have to keep rewriting those methods while the API is changing. And imagine what would have happened if I push 10+ packages to the Inbox, noone would have taken the time to review them thoroughly, even these three seems to be too much.
My main objection is popping Squeak UI-elements from directly within the domain. If you don't like Utilities to be the bridge between the Squeak UI and the domain, we can address that next. But that is independent of having Author be a pure domain.
I think the instance side is the domain, the class side has nothing to do with it.
Utilities is an "enemy" of modularity, because it's a bunch of unrelated methods. I guess it started out as a class to hold utility methods. The problems started when they got senders from the image and the situation got worse when state was added to it.
Every user of Utilities will depend on it's package and Utilities depend on everything it uses (a bunch of packages), so if you use only one thing from it (e.g. author information) then your package will depend on a bunch of other stuff totally unrelated to your package.
There's also the notion of having multiple instances instead of the global-variable approach -- are you ok with that?
My original idea was to simply break out the author related stuff from Utilities into a single class. Since in the current system there's only a single author information maintained it was a straightforward idea to make Author a singleton. Colin proposed that multiple instances might be useful, though I still don't see any real world use cases for it, but I'm not against not overriding #new.
Testing that an author's name is correctly added when saving a method sounds like a good case for allowing multiple instances. (Or more generally, any kind of test around recording authorship.)
frank
Levente
On Fri, Sep 14, 2012 at 10:41 PM, Levente Uzonyi leves@elte.hu wrote:
On Fri, 14 Sep 2012, Chris Muller wrote:
Hi guys! Levente, thanks for pushing this forward!
My feeling is that "Author current" should be the only place where we lazily initialize by asking the user. Then Author>>initials would just be a normal accessor, and Author class>>initials would go through #current. The tricky bit with that idea is that we'd need a slightly more elaborate UI, so let the user fill in both initials and username.
One idea would be to get rid of #username for now, and have Author objects with just initials. Then in 4.5 we we'd flesh things out a bit more.
The way these methods behave is similar to how Utilities works. The *PerSe methods are just simple accessors and the accessor looking methods ensure that they are initialized.
What about removing the *PerSe methods from the instance side and update boths sides so that only the class side accessor looking methods request the initialization of the data? E.g.:
Author initials. "this will request the initials if they are not set" Author current initials. "just returns the initials" Author initialsPerSe. "just returns the initials" Author current intialsPerSe "MNU"
This would make the behavior of the instance and the class side different, which might be surprising.
Another idea is to not try to make the new implementation resemble the old one, just ensure the compatibility via the methods in Utilities.
Say, would y'all mind if we think of Author as a *pure domain object* and NOT couple any Squeak-specific UI behaviors to it whatsoever? We all know doing that is bad practice so how about letting Utilities take responsibility for ensuring a there is a well-formed "current Author".
That frees Author to just "be" a simple and regular kind object of which there can be multiple instances. Its just that the "current" one is the one which the rest of the system recognizes as the one authoring (saving code, etc).
Real people's names don't usually change, so let's not treat "Author current" like a global variable and temporarily maul someone's name just to accomodate some test process. Besides, then we'd have to worry about Mutexes and multi-process updating, etc..
I submitted an update to the inbox that takes this tact. With this, applications that are ok to be integrated with Squeak UI elements can use:
Utilities authorInitials
and it will make sure the Author current initials is set. Meanwhile, applications that just want to access the current value and possibly set it in their own way can simply check
Author current initials
and take action in their own way if they want.
What do you think?
I don't like the idea to keep methods about author in Utilities. My plan is/was to rewrite all users in the image when the Author implementation is there and deprecate all author related methods in Utilities. So IMHO the class side of Author is a much better place for those methods.
Levente
On Sun, 16 Sep 2012, Frank Shearar wrote:
On 16 September 2012 02:19, Levente Uzonyi leves@elte.hu wrote:
My original idea was to simply break out the author related stuff from Utilities into a single class. Since in the current system there's only a single author information maintained it was a straightforward idea to make Author a singleton. Colin proposed that multiple instances might be useful, though I still don't see any real world use cases for it, but I'm not against not overriding #new.
Testing that an author's name is correctly added when saving a method sounds like a good case for allowing multiple instances. (Or more generally, any kind of test around recording authorship.)
Creating another instance is still possible (#shallowCopy), I wrote the tests that way, so they remain future proof in case someone adds more data to Author.
Levente
frank
Levente
On Fri, Sep 14, 2012 at 10:41 PM, Levente Uzonyi leves@elte.hu wrote:
On Fri, 14 Sep 2012, Chris Muller wrote:
Hi guys! Levente, thanks for pushing this forward!
My feeling is that "Author current" should be the only place where we lazily initialize by asking the user. Then Author>>initials would just be a normal accessor, and Author class>>initials would go through #current. The tricky bit with that idea is that we'd need a slightly more elaborate UI, so let the user fill in both initials and username.
One idea would be to get rid of #username for now, and have Author objects with just initials. Then in 4.5 we we'd flesh things out a bit more.
The way these methods behave is similar to how Utilities works. The *PerSe methods are just simple accessors and the accessor looking methods ensure that they are initialized.
What about removing the *PerSe methods from the instance side and update boths sides so that only the class side accessor looking methods request the initialization of the data? E.g.:
Author initials. "this will request the initials if they are not set" Author current initials. "just returns the initials" Author initialsPerSe. "just returns the initials" Author current intialsPerSe "MNU"
This would make the behavior of the instance and the class side different, which might be surprising.
Another idea is to not try to make the new implementation resemble the old one, just ensure the compatibility via the methods in Utilities.
Say, would y'all mind if we think of Author as a *pure domain object* and NOT couple any Squeak-specific UI behaviors to it whatsoever? We all know doing that is bad practice so how about letting Utilities take responsibility for ensuring a there is a well-formed "current Author".
That frees Author to just "be" a simple and regular kind object of which there can be multiple instances. Its just that the "current" one is the one which the rest of the system recognizes as the one authoring (saving code, etc).
Real people's names don't usually change, so let's not treat "Author current" like a global variable and temporarily maul someone's name just to accomodate some test process. Besides, then we'd have to worry about Mutexes and multi-process updating, etc..
I submitted an update to the inbox that takes this tact. With this, applications that are ok to be integrated with Squeak UI elements can use:
Utilities authorInitials
and it will make sure the Author current initials is set. Meanwhile, applications that just want to access the current value and possibly set it in their own way can simply check
Author current initials
and take action in their own way if they want.
What do you think?
I don't like the idea to keep methods about author in Utilities. My plan is/was to rewrite all users in the image when the Author implementation is there and deprecate all author related methods in Utilities. So IMHO the class side of Author is a much better place for those methods.
Levente
Your version left just as many methods about author in Utilities as mine. We're not addressing that right now because we're moving forward gently and incrementally.
If you check those methods, all logic was removed from them, they just perform a send to Author. Btw there's a reason why I uploaded 2 packages. They have to be loaded one after the other. The first creates the Author and initializes the instance with the data from Utilities, the second updates the methods in Utilities, so the system will indirectly use Author. Loading this stuff in a single step might have unexpected side effects (e.g. loss of author information).
I didn't remove the methods from Utilities because I want to deprecate them first, but before they are deprecated, the senders from the Trunk packages have to be rewritten. I didn't want to rewrite a bunch of methods while the API is not stable, because I'd have to keep rewriting those methods while the API is changing. And imagine what would have happened if I push 10+ packages to the Inbox, noone would have taken the time to review them thoroughly, even these three seems to be too much.
Yip, I totally got that.
My main objection is popping Squeak UI-elements from directly within the domain. If you don't like Utilities to be the bridge between the Squeak UI and the domain, we can address that next. But that is independent of having Author be a pure domain.
I think the instance side is the domain, the class side has nothing to do with it.
That's fine -- I'm ok with Author *class* taking responsibility for the initializing the global "current" Author of the System including popping Squeak UI's to handle that initialization. I'm not ok with having that happen in the lazy-initters on the instance-side and then asking outsiders who want to use the domain use PerSe accessors.
It seems like you're ok with my not-ok. :)
Utilities is an "enemy" of modularity, because it's a bunch of unrelated methods. I guess it started out as a class to hold utility methods. The problems started when they got senders from the image and the situation got worse when state was added to it.
Every user of Utilities will depend on it's package and Utilities depend on everything it uses (a bunch of packages), so if you use only one thing from it (e.g. author information) then your package will depend on a bunch of other stuff totally unrelated to your package.
You don't need to defend this. If I gave the impression that I wanted to keep Utliities then I miscommunicated. The remaining author-methods are there only for the temporary-compatibility / smooth-transition reasons you said above. In the meantime I thought we would consider Compiler or SmalltalkImage or Tools or some other external manipulator but I agree Author class is just as good if not the best choice.
There's also the notion of having multiple instances instead of the global-variable approach -- are you ok with that?
My original idea was to simply break out the author related stuff from Utilities into a single class. Since in the current system there's only a single author information maintained it was a straightforward idea to make Author a singleton. Colin proposed that multiple instances might be useful, though I still don't see any real world use cases for it, but I'm not against not overriding #new.
I understand you. But cool, I'm glad you're ok with that. I think we do have a use-case today; "Change the current Author". We've taken the trouble to factor out a first-class Author so it's natural to want to think of it in real-world terms. Authors are people whose names don't usually change so it feels like a... "violation" to do that use-case by updating the inst-vars of the current Author. Instead the instance should be swapped and there's no reason to block #new.
On 2012-09-16, at 03:19, Levente Uzonyi leves@elte.hu wrote:
I didn't remove the methods from Utilities because I want to deprecate them first, but before they are deprecated, the senders from the Trunk packages have to be rewritten.
+1
We tried proper deprecation in the past and it were not that thorough, but we should try again. That is, move deprecated classes/methods into a "Deprecated-in-4.4" package and keep that loaded by default at least for one more release, but remove it at some point.
Colin proposed that multiple instances might be useful, though I still don't see any real world use cases for it, but I'm not against not overriding #new.
The one use case I encounter quite often is temporarily setting different initials for generated methods. Might be useful for tests too.
- Bert -
On 17 September 2012 13:55, Bert Freudenberg bert@freudenbergs.de wrote:
On 2012-09-16, at 03:19, Levente Uzonyi leves@elte.hu wrote:
I didn't remove the methods from Utilities because I want to deprecate them first, but before they are deprecated, the senders from the Trunk packages have to be rewritten.
+1
We tried proper deprecation in the past and it were not that thorough, but we should try again. That is, move deprecated classes/methods into a "Deprecated-in-4.4" package and keep that loaded by default at least for one more release, but remove it at some point.
Enforcing the deprecation's relatively easy: we need just add a step to releasing a version N that removes the deprecated packages for N-2, N-3, ... That way we'd keep one round of deprecations.
The "need to be more thorough" bit I guess is what you mean, in particular: marking something as deprecated will still be manual.
frank
Colin proposed that multiple instances might be useful, though I still don't see any real world use cases for it, but I'm not against not overriding #new.
The one use case I encounter quite often is temporarily setting different initials for generated methods. Might be useful for tests too.
- Bert -
On Fri, 14 Sep 2012, Colin Putney wrote:
On Fri, Sep 14, 2012 at 10:36 AM, Levente Uzonyi leves@elte.hu wrote:
Another idea is to not try to make the new implementation resemble the old one, just ensure the compatibility via the methods in Utilities.
My feeling is that "Author current" should be the only place where we lazily initialize by asking the user. Then Author>>initials would just be a normal accessor, and Author class>>initials would go through #current. The tricky bit with that idea is that we'd need a slightly more elaborate UI, so let the user fill in both initials and username.
That doesn't seem to be too hard to do by checking both the initials and the name and ask for both one after the other if they are missing.
One idea would be to get rid of #username for now, and have Author objects with just initials. Then in 4.5 we we'd flesh things out a bit more.
I wanted to separate Author from Utilities, but if we leave #authorName in utilities, then it won't happen in 4.4.
I used #name on the instance side first, but I decided to unify the two sides. If we decide to drop the class side convenience methods, then we can use #name.
Ah, that makes sense. But how about #fullName? For me, #username implies some kind of account somewhere, either the "current user" from the OS, SqueakMap, SqueakSource or the like.
Pharo uses #fullName for what we use #initials for. If we don't want our API to be compatible at this level, then we can use #fullName instead of #username.
Levente
Colin
On Fri, Sep 14, 2012 at 9:13 PM, Levente Uzonyi leves@elte.hu wrote:
My feeling is that "Author current" should be the only place where we lazily initialize by asking the user. Then Author>>initials would just be a normal accessor, and Author class>>initials would go through #current. The tricky bit with that idea is that we'd need a slightly more elaborate UI, so let the user fill in both initials and username.
That doesn't seem to be too hard to do by checking both the initials and
the
name and ask for both one after the other if they are missing.
But remember, we'd be checking for the presence of an author object, and creating one from scratch, rather than checking each field separately. So we'd pretty much always have two dialogs in succession, which is a bit awkward. I suppose we could do a single dialog with a template like:
Author username: 'Your Name' initials: 'yn'.
That's how most other tools work.
I just think that if we continue to treat initials and name as separate entities that can be set and queried independently, then there's not much point doing the extraction at all. We're just changing where the globals values are stored.
One idea would be to get rid of #username for now, and have Author objects with just initials. Then in 4.5 we we'd flesh things out a bit more.
I wanted to separate Author from Utilities, but if we leave #authorName in utilities, then it won't happen in 4.4.
Oh wow, I didn't even know we *had* authorName in Utilities. In that case, extracting it along with #authorInitials makes perfect sense. Strike my suggestion.
Pharo uses #fullName for what we use #initials for. If we don't want our
API
to be compatible at this level, then we can use #fullName instead of #username.
Right, let's not introduce gratuitous incompatibility. How about #properName?
Colin
squeak-dev@lists.squeakfoundation.org
-
Bert Freudenberg -
Chris Muller -
Chris Muller -
Colin Putney -
Frank Shearar -
H. Hirzel -
Levente Uzonyi