Extracing author information from Utilities

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

Extracing author information from Utilities

Levente Uzonyi-2
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

Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Colin Putney-3
On Fri, Sep 14, 2012 at 9:04 AM, Levente Uzonyi <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Levente Uzonyi-2
On Fri, 14 Sep 2012, Colin Putney wrote:

> On Fri, Sep 14, 2012 at 9:04 AM, Levente Uzonyi <[hidden email]> 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.
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.

>
> 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.

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.

>
> 3. 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
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Colin Putney-3
On Fri, Sep 14, 2012 at 10:36 AM, Levente Uzonyi <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Chris Muller-3
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?

Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Hannes Hirzel
On 9/14/12, Chris Muller <[hidden email]> 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?
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Levente Uzonyi-2
In reply to this post by Chris Muller-3
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


Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Levente Uzonyi-2
In reply to this post by Colin Putney-3
On Fri, 14 Sep 2012, Colin Putney wrote:

> On Fri, Sep 14, 2012 at 10:36 AM, Levente Uzonyi <[hidden email]> 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
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Colin Putney-3
In reply to this post by Levente Uzonyi-2
On Fri, Sep 14, 2012 at 8:41 PM, Levente Uzonyi <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Colin Putney-3
In reply to this post by Levente Uzonyi-2


On Fri, Sep 14, 2012 at 9:13 PM, Levente Uzonyi <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Chris Muller-4
In reply to this post by Levente Uzonyi-2
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 <[hidden email]> 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
>

Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Levente Uzonyi-2
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 <[hidden email]> 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
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Frank Shearar-3
On 16 September 2012 02:19, Levente Uzonyi <[hidden email]> 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 <[hidden email]> 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
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Levente Uzonyi-2
On Sun, 16 Sep 2012, Frank Shearar wrote:

> On 16 September 2012 02:19, Levente Uzonyi <[hidden email]> 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 <[hidden email]> 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
>>>>
>>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Chris Muller-4
In reply to this post by Levente Uzonyi-2
>> 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.

Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Bert Freudenberg
In reply to this post by Levente Uzonyi-2

On 2012-09-16, at 03:19, Levente Uzonyi <[hidden email]> 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 -



Reply | Threaded
Open this post in threaded view
|

Re: Extracing author information from Utilities

Frank Shearar-3
On 17 September 2012 13:55, Bert Freudenberg <[hidden email]> wrote:
>
> On 2012-09-16, at 03:19, Levente Uzonyi <[hidden email]> 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 -
>
>
>