Perl-Apollo / Corinna

Corinna - Bring Modern OO to the Core of Perl
Artistic License 2.0
157 stars 19 forks source link

Rationale Feedback #3

Closed Ovid closed 3 years ago

Ovid commented 4 years ago

Use this issue to leave feedback on the Cor Rationale.

openstrike commented 4 years ago

Is it too late to change the name?

The fact that it sounds like "core" is a happy coincidence.

We already have some (thankfully limited) confusion between core and CORE. To now add Cor to the mix isn't going to help. Try explaining to someone new to Perl that Cor is in core but not in CORE and they are most likely to run screaming back to python.

Ovid commented 4 years ago

@openstrike This has complicated some video calls I've had with Sawyer and Stevan, so I know where you're coming from. When that happens, we just refer to it by its full name: Corinna.

openstrike commented 4 years ago

Indeed, "Corinna" would be much less ambiguous. Thanks.

haarg commented 4 years ago

The Goals section doesn't match what is stated on the home page.

matthewpersico commented 4 years ago

My apologies for not chiming in sooner, but I've been surprisingly busier than when I had to commute for a living. This comment will deal with this code:

    has $cache :handles(get) :builder;
    method _build_cache () { Hash::Ordered->new }

More to follow as I read.

matthewpersico commented 4 years ago
  has $cache :handles(get) = Hash::Ordered->new;

To me, this says "the cache slot handles gets by calling Hash::Ordered->new". Obviously that's wrong. It's supposed to mean that the cache slot is initialized with a call to Hash::Ordered->new. This looks too much like Python's bolted on mypy nonsense sprinkling type info in between declaration and assignment. Please keep the things that are related visually textually together:

  has $cache = Hash::Ordered->new :handles(get);

If that's a parsing nightmare, then maybe

  has $cache :initer(Hash::Ordered->new) :handles(get);
matthewpersico commented 4 years ago
has $created :reader = time;

_In the above, we have a new :reader attribute which says "create a read-only method name created to read this value. But if you want a different method name, that's fine: :reader(getcreated).

See? In https://github.com/Ovid/Cor/issues/3#issuecomment-636976258, I guessed readers were defined as *get*_slot() and not slot() I was wrong...

matthewpersico commented 4 years ago

You don't see it in the above code, but you automatically get a $self variable in a method if you declare it with method.

Every OO language I know has some kind of explicit 'self' expression. I wonder how many people are going to blanche at not being able to tell their slots from other variables at first glance without a $self and do (something_like) this:

class Customer {
    has $s_name      :isa(Str)      :new :reader(name) :writer(name);
    has $s_birthdate :isa(DateTime) :new :reader(birthdate) :writer(birthdate);
}

where s_ is the stand-in for self.

duncand commented 3 years ago

Why call it version v0.01.0 rather than v0.1.0? Multi-dot notation is explicitly a series of integers for whom leading zeroes don't change the meaning. If you actually want to overlay with Perl's native floating point number versions, then it would be 0.001000.

Ovid commented 3 years ago

@duncand Version switched to v0.1.0. That's just me being silly. Thanks for catching that.

duffee commented 3 years ago

The assertion that Cor is better than Moo/se is missing supporting arguments beyond performance/core-ness (i.e. non-syntax issues). IMHO, the short class definition (I mean the Constructor attributes ) is not going to win any converts to Perl because it is too dense. If we as readers didn't need white space, there would be no /x regex modifier. The syntax for Corinna is great for writing, horrible for reading because it ends up being a wall of text. So the question is, who is your audience? Experienced perlers or new converts?

My suggestion for post-lockdown is to temp some Javascript developers with coffee and pastry to do some A/B/C testing. They won't freak out over $variables, they haven't used Moo/se before and people will agree to anything when pumped up on caffeine and sugar. Show them a simple class with a view attributes, a couple of clear methods and a frobobnicate method written up in bless, Moose and Corinna style as you've done in the Rationale. Ask them what the frobobnicate method does just to get them to engage with the class and then get them to rank the styles in order of clarity. Print the classes out on separate pieces of paper so they can be viewed side by side and shuffled around or scribbled on.

Both bless and frobobnicate are distractors. You're trying to warm up your subjects so they relax and can give you their gut instinct on the syntax. You don't want them to overthink the differences between Moose and Corinna or you'll get bad data. My guess is that Moose will rank highest for clarity, but what if the sweet spot is somewhere between? Well then, time for A/B/C/D/E testing which is starting to resemble the Card sorting method of elicitation. Take a single feature from one syntax and drop it in the other so that you can identify the best combination of features.

All of this side steps the issue of how easy it is to maintain, but with data, you stand a better chance of convincing p5p.

duncand commented 3 years ago

One thing that's super-important when asking opinions on syntax is to use examples that are as realistic as possible, the kinds of classes that would be commonly written in real work intended for a real production system.

So none of that "Point" class stuff having artificially short member names and no practical use.

It would need to have member names with realistically descriptive lengths and quantities and variety.

Ovid commented 3 years ago

@duffee wrote:

IMHO, the short class definition (I mean the Constructor attributes ) is not going to win any converts to Perl because it is too dense. If we as readers didn't need white space, there would be no /x regex modifier. The syntax for Corinna is great for writing, horrible for reading because it ends up being a wall of text.

Then add whitespace.

class Cache::LRU {
    use Hash::Ordered;

    has $cache
        :handles(get)
        = Hash::Ordered->new;

    has $max_size
        :new
        :reader
        = 20;

    has $created
        :reader
        = time;

    method set ( $key, $value ) {
        if ( $cache->exists($key) ) {
            $cache->delete($key);
        }
        elsif ( $cache->keys > $max_size ) {
            $cache->shift;
        }
        $cache->set( $key, $value );  # new values in front
    }
}

Seriously, I'm unsure if I'm missing the point or not. If is => 'rwp' is better than :reader because it has more whitespace, can you explain why? Why is handles => [ qw/ get set / ] better than :handles( get, set )?

Or an example similar to what I use in talks:

has volume => (
    is       => 'ro',
    init_arg => undef,
    builder  => '_build_volume',
);

sub _build_volume($self) {
    return $self->height * $self->width * $self->depth;
}

Why is the above preferable to has $volume :reader = $height * $width * $depth; other than as a "style" thing?

Or you can do:

has $volume
    :reader
    = $height * $width * $depth;

I'm not trying to be difficult. I genuinely don't understand the issue here (also, post-lockdown is likely a year or more away).

duncand commented 3 years ago

Right on Ovid, I find your Corinna version nicer than the Moo* ones.

abraxxa commented 3 years ago

I prefer Moo(se) because the key/value pairs are better readable.

Ovid commented 3 years ago

This is resolved for the MVP. Further issues should be new tickets.