On Fri, 14 Sep 2012, Colin Putney wrote:

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

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