Perl-Apollo / Corinna

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

Classes system feedback #103

Open hurricup opened 1 year ago

hurricup commented 1 year ago

Some background:

So my feedback is kinda view from the outside and it based on current implementation in 5.38, not all propositions in this particular repo (sorry, did not have an opportunity to read all of them). I wrote some of my concerns to the p5p and was suggested to submit them here, so - here I am.

So here are my 5 cents:

  1. I don't see the way to pass a fixed value/derived value as a parameter to a superclass constructor without necessity to pass it to the subclass. In theory I can do that in the ADJUST phaser if superclass provides a setter for it, but there is no guarantee and this won't work if parent ADJUST relies on that field being set in construction. Here is java-like example:
    
    class ParentClass{
    private final String myParam;
    public ParentClass(String param){
       myParam = param;
    } 
    }

class ChildClass extends ParentClass{ public ChildClass(){ super("someFixedValue"); } }


3. I don't see the mechanism to make constructor parameters mandatory and I need to write checks myself in the ADJUST phaser, which feels redundant and something that could be declarative/automated. Like:
```perl5
field $something :mandatory;

# or

field $otherthing :mandatory { 
   #validator code 
};
  1. Classes versioning. I know that perl has this as long as I used it, but is it really necessary and should be used for some modern system? Can I have 2 classes with the same name and different versions and inherit one of them by specifying version? I guess not. Feels like there are easier ways to set a version field/global variable. And if anyone besides dependency manager needs to check for a version - they can do it explicitly by checking variable or using a getter. Also i see that in this repo version is suggested to be set via attribute, but 5.38 introduces version as a first class citizen as a part of base syntax (I presume this is a consequence of having existing parsing branches for this from package)
  2. Inheritance syntax. Feels like using attributes for the thing we almost always have is too verbose and would be better to have something more concise for this:
    
    class First::Class : Parent::Class;

instead of

class First::Class :isa(Parent::Class);

I understand historical reasons and syntax continuity, but feels like modern stuff should be modern and easy to use, and less follow historical syntax rules without a really good reason for language users.
7. I'd like to be able to declare multiple fields with one instruction:
```perl5
field $one, $two, $three :param;
field $four = 42,
        $five = 'something';

# or 

field ($four, five) = (42, 'something'); # but previous version feels more readable
  1. Would be really nice to be able to use positional arguments for the constructor. As far as we don't have a native named params support and we don't want to introduce them with this task, we could try to invent some convention here, but this feels like a really dangerous way to go.

Again, sorry if this was already discussed/described in this repo or p5p.

HaraldJoerg commented 1 year ago
  1. Passing a fixed value/derived value as a parameter to a superclass constructor: This has indeed been discussed (but hard to spot in the long discussion) and I'd say it is one of the known rough areas. Here's what I settled for:
use 5.038;
use feature 'class';
no warnings 'experimental';

class ParentClass{
    field $myParam :param(param) = undef;
    ADJUST {
    $myParam //= $self->initParam();
    }
    method initParam {
    die "Subclasses are supposed to override this";
    }
    method myParam { $myParam }
}

class ChildClass :isa(ParentClass) {
    method initParam () {
    return "someFixedValue";
    }
}

say ChildClass->new->myParam;

This has a quirk: I need a public method initParam which is not supposed to be part of the class API. I guess some future version of Corinna will improve on that, as several declarative ways have been discussed, Alas, it's not in Perl 5.38 (and not in Object::Pad either).

  1. A mechanism to make constructor parameters mandatory: A :param without a default denotes a mandatory parameter. In our example above, this can not be used even if ParentClass would like to make the parameter mandatory - another quirk of that implementation.

  2. Version syntax and use: As far as I know it is indeed re-using the parser Perl has for package.

  3. Inheritance syntax: Be careful with the colon: There be dragons. For a start, besides inheritance there's the use of roles (not yet implemented in Perl, but available with Object::Pad), and they are declared as :does(Some::Interesting::Stuff). Besides, this also "suffers" from the somewhat quirky Perl parsing of attributes: You can have spaces between the colon and the attribute name, and if you have more than one attribute, then only the first one needs a colon. If you declare inheritance with a bare colon and that colon is sort of optional, the specification gets murky.

  4. Positional arguments for the constructor: This gets messy to declare when a class and its parent class both have constructor params. My workaround for this is to have custom constructors.

...Perhaps I should add some background, too: I'm one of the maintainers of Perl 5 support for Emacs, which as of now covers feature 'class'.

perigrin commented 1 year ago

So there is a problem with:

class ParentClass{
   private final String myParam;
   public ParentClass(String param){
       myParam = param;
   } 
}

class ChildClass extends ParentClass{
   public ChildClass(){
        super("someFixedValue");
   }
}

The problem is that you can't use ChildClass in all places ParentClass is used. Specifically you have to re-write any constructors to no-longer take param after a swap. In a simple example like this, it isn't a huge problem, but it does lead to some weird corners as complexity scales and honestly I would generally use composition over inheritance in this cases.


class ParentClass {
     field $param :param;
     method param() { $param }
}

class ChildClass {
      field $parent = ParentClass->new(param => "someFixedValue");
      method param() { $parent->param() } 
}

In my experience anything sufficiently complicated with inheritance becomes a pain in the ass. Most often I'm gonna want to subclass ChildClass but with a different value of "someFixedValue" … or I'm going to want an instance of ChildClass but I need a different value for "someFixedValue" but the implementation of ChildClass is tightly bound to that value always being "someFixedValue", so I have to entirely re-implement ChildClass with that new value.

If you want some kind of contractual constraint, eventually we'll have Roles that you can use to define an interface that requires method param().

Regarding 5, in addition to what @HaraldJoerg said … I disagree that Inheritance is a "thing we almost always have".[^1] I'd rather it be a thing we almost never have. I'm not sure I've yet found a place where I preferred it over role composition or even better, simple delegation (like above). (This is despite my having argued for it already in other tickets/discussions in this same repo.) I strongly suspect it was a violation of Ovid's Rule of Parsimony to even have it at all in the current code but it's a feature that's so deeply ingrained into Perl's OO culture that I doubt we could get away without it.

Finally 6, … this goes back to the same problems you were having with 1 I think. Perl injects a constructor implicitly, you never actually define one. That said eventually we will have static methods in classes that you can do something like method new_from_list :common ($a, $b, $c) { $class->new(a => $a, b => $b, c => $c) } and you can currently implement that by simply using a subroutine in the class:

class Animal {
  field $name :param;

  ADJUST { say "name: $name" }

  sub new_with_name($class, $name) { $class->new(name => $name) }
}

my $a = Animal->new_with_name("Fido");

Which is basically how you'd implement the same idiom in Moose or Moo.

[^1]: I've managed to implement a pretty good chunk of a video game without using inheritance.