Perl-Apollo / Corinna

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

Constructor Feedback #1

Closed Ovid closed 3 years ago

Ovid commented 4 years ago

Use this issue to provide feedback on the Cor Constructor Proposal.

Abigail commented 4 years ago

I've been bothered with the lack of proper constructors in Perl for a long time. Until I realized Perl does have a proper constructor, (bless), but the problem lies in what people put in new: they both construct and initialize the object in the same method. So, for more than a decade, all my new methods do is call bless, and actual initialization in a separate method called init (this makes multiple inheritance way easier as a side effect).

This is how I would write the Box/Cube example:

#!/opt/perl/bin/perl

use 5.026;

use strict;
use warnings;
no  warnings 'syntax';

use experimental 'signatures';

package Box {
    use Hash::Util::FieldHash qw [fieldhash];

    fieldhash my %height;
    fieldhash my %width;
    fieldhash my %dept;

    sub new {bless \do {my $var} => shift};

    sub init ($self, $height, $width, $dept) {
        $height {$self} = $height;
        $width  {$self} = $width;
        $dept   {$self} = $dept;

        $self;
    }

    sub volume ($self) {
        $height {$self} * $width {$self} * $dept {$self}
    }
}

package Cube {
    our @ISA = qw [Box];

    sub init ($self, $height) {
        $self -> SUPER::init (($height) x 3);
    }
}

my $cube = Cube:: -> new -> init (7);
say $cube -> volume;

__END__

Alternatively, I could make use of Perls signatures, and just default the width and the depth by writing

sub init ($self, $height, $width = $height, $dept = $height) { ... }

but that gives less flexibility than a tiny subclass.

dr-kd commented 4 years ago

There are some hard problems here. I think BUILDARGS works reasonably well because of the principle that "sufficiently encapsulated ugly is indistinguishable from beautiful" although I don't think BUILDARGS is quite sufficiently encapsulated.

happy-barney commented 4 years ago

I'd like to share use case from usage of raku's Grammar where every rule / token creates method with same name. I tried to implement SQL parser with some reasonable readability

Limitations:

but CREATE method is not necessary to be accessible all the time, it makes sense only in object construction phase.

It will be nice to be able to specify any special behaviour via dsl

something like:

class ... {
   has x ... => (
      build_from => {
         y => sub { $_[0]->{y} + $_[0]->{z} },
     },
   );

   init argument y => (
     isa => 'Int',
     requires => 'z', # when y is specified, 'z' is also mandatory
   ),

   init argument foo => (
     provides => {
        y => sub { $_[0]->y },
        z => sub { $_[0]->z },
     }
   )
}
cfedde commented 4 years ago

What concept hangs on the new: attribute on a slot name? In what case would it not be used? Would it ever be used on a slot that has behavior? It seems to me that a word with more semantic load might be better. Or just factor it out completely. and just use required:, optional:, forbiden: without the extra decoration. Obviously required: would be the default.

has ($width, $height, $depth)  :reader :isa(Num);

Just a thought.

Ovid commented 4 years ago

@cfedde has $var only declares the slots. Absolutely nothing else. That has nothing to do with whether or not we want this to be passed via the constructor. The :new(...) attribute provides that. Further, it's a single attribute with several optional values to ensure it's composable. By making :required and :optional separate attributes, they could accidentally both be supplied to the same slot, thus generating an error. We're trying to make it harder to write incorrect code, so non-composability needs to be minimized.

In what case would it not be used?

Plenty of cases where an object needs to track data internally without exposing it.

cxw42 commented 4 years ago

Can we assume multiple dispatch based on signatures? As you pointed out on the constructors page, Java (and C++) provide overloaded constructors. If we had that option, it might ease handling some of the cases that currently use BUILDARGS.

Ovid commented 4 years ago

@cxw42 We won't get multiple dispatch.

HaraldJoerg commented 4 years ago

Will it still be possible to call new as an object method on an existing object? "Traditional" Perl OO and Moo* allow that, with a rather generic meaning of "make the new object from the same class as the invocant", and I've seen it being used on CPAN.

Otherwise callers need to call ref($object)->new instead of just $object->new, so it's no big deal, just convenience and habit. edit: The page "deconstructing constructors" is still referenced, but seems to be gone.

iamalnewkirk commented 4 years ago

I can't imagine releasing a ground-up modern OO system that doesn’t throw exceptions, and similarly, it would be odd to have exceptions without try/catch/finally support. Assuming we’ll have exceptions and the ability to catch them, I think the constructor method NEW should collect exceptions and throw an aggregate exception representing all failures as opposed to throwing on the first error encountered.

duncand commented 3 years ago

I agree with the fundamental constructor syntax being exclusively named arguments, please keep that!

leonerd commented 3 years ago

I haven't seen it discussed elsewhere so I'll ask this question here: How are we going to deal with named constructor arguments that don't translate into slot names?

For example, I have a class currently defined with this:

class Tickit::Widget::FloatBox::Float;

has $_floatbox;
has $_child;
has $_hidden;

BUILD (%args) {
  $_floatbox = delete $args{floatbox};
  $_child    = delete $args{child};
  $_hidden   = delete $args{hidden} || 0;

  $self->move( %args );
}

I.e. pull out a few named arguments to set initial values of slots, then invoke the ->move method to set the initial position of the float. This method takes a bunch of named args like "top", "bottom", etc... which aren't necessarily just more slot names.

my $float = Tickit::Widget::FloatBox::Float->new(
  floatbox => $floatbox,
  child => $widget,
  top => 3, bottom => -3, left => 6, right => -6,
);

If this were rewritten using the :param (or :new or whatever we're going to call it) syntax, it begins to look like this:

class Tickit::Widget::FloatBox::Float;

has $_floatbox :param;
has $_child :param;
has $_hidden :param = 0;

ADJUST (%args) {
  $self->move( %args );
}

but now how is this going to work? How do we tell the base layer that is checking the names of all the constructor params, to allow "top", "bottom", etc.. through the constructor so they can hit that initial ->move?

leonerd commented 3 years ago

Sudden thought: Could the BUILD or ADJUST block take a :param(...) attribute to suggest what other named args it will take?

class Tickit::Widget::FloatBox::Float;

has $_floatbox :param;
has $_child :param;
has $_hidden :param = 0;

ADJUST :param(top,bottom,left,right) (%args) {
  $self->move( %args );
}

There's a certain neatness to this - the sum total of all the :param names across all the slots and BUILD/ADJUST blocks becomes the set of named parameters the constructor takes (where slots default to their own slotname if not otherwise specified)

tm604 commented 3 years ago

This wouldn't be able to replace the current bless-based syntax if it requires named parameters. As default behaviour, sure - but there should also be a way to override this.

Examples of problematic classes would include ORM wrappers or https://metacpan.org/pod/String::Tagged.

duncand commented 3 years ago

Examples of problematic classes would include ORM wrappers or https://metacpan.org/pod/String::Tagged.

What is special about ORM wrappers that require non-named constructor parameters?

duncand commented 3 years ago

This wouldn't be able to replace the current bless-based syntax if it requires named parameters. As default behaviour, sure - but there should also be a way to override this.

I didn't get the impression that Corinna based classes were intended to be able to 100% emulate bless-based classes and thus some module Foo version 2 implemented over Corinna being a drop-in replacement for Foo version 1 implemented over bless.

So unless that's a design goal of Corinna, I would say anyone adopting Corinna would have to assume new versions of classes may not be drop-in compatible and users of those classes may have to tweak their code to keep working with them, or they don't upgrade.

duncand commented 3 years ago

It seems to me that in order to properly support the option of named or positional arguments, Perl needs to have an unambiguous calling syntax for routines in general that can distinguish positional and named arguments. Plain foo(a,b,c,d) or foo(a=>b,c=>d) doesn't do the job, because => is just a comma, nothing inside foo can tell which of those it was called with. If we solve this first, then we can support optional positional constructor arguments, or mixtures of positional and named arguments, anywhere.

Ovid commented 3 years ago

For v1, named arguments that don't correspond to slot names causes a runtime error per section 4 of the constructor logic. This is to prevent this common bug:

class Foo {
    has $field :new;
    ...
}

my $foo = Foo->new( feild => $value );

but now how is this going to work? How do we tell the base layer that is checking the names of all the constructor params, to allow "top", "bottom", etc.. through the constructor so they can hit that initial ->move?

The current plan to to supply alternate constructors that wrap new and then do anything else you need.

common method new_position (%args) {
     my $floatbox = delete $args{floatbox};
     my $child    = delete $args{widget};
     my $instance = $class->new( floatbox => $floatbox, child => $child );
     $instance->move(%args);
     return $instance;
);
Ovid commented 3 years ago

@leonerd wrote:

Sudden thought: Could the BUILD or ADJUST block take a :param(...) attribute to suggest what other named args it will take?

Remember that we don't have BUILD any more. It's all alternate constructors.

It's interesting, but could that be a future RFC? That will complicate the constructor logic because we'd have to now grab the params listed on ADJUST blocks throughout the inheritance hierarchy and ensure we have no conflicts, but that they're ignored in section 4, but passed to ADJUST in section 6.

haarg commented 3 years ago

With the current model, it seems pointless to pass the arguments in to ADJUST, since all arguments already have to correspond to slots, so they will already be stored in the object and accessible via slot variables.

Ovid commented 3 years ago

@haarg Agreed. If arguments get passed to ADJUST, it's no longer a normal phaser the way we envisioned it.

Ovid commented 3 years ago

At this point, this issue appears to largely be settled for the MVP.