Trusted methods

[Ovid]

A quick reminder about encapsulation. It's not about hiding state. In OOP, to the outside world a class is nothing more than a complex type, an expert about a particular problem domain. The implementation is opaque. This is why a web server is a perfect "object": your browser generally can't screw around with its internals (barring security issues).

Externally a class is an expert, but internally that expertise is managed by coupled state and process. So for this discussion, we'll want to avoid temptations to just export state to other classes. It needs to be state plus process.

---

In a project where I'm using use feature 'class'; to look see how we're doing, I defined a field $token :param (which takes an array reference) in an abstract base class. Six concrete children can all be instantiated as $class->new( token => $token ) and need access to this data, so I had to provide method _get_token {$token} in the base class. This means anything outside the class could get access to the state, violating encapsulation. I resolved this by hard-coding the following in all six concrete classes.

field $token :param;

Obviously, cutting-n-pasting is not good. If I later need to add additional constructor constructor parameters, I'd have to cut-n-paste a lot of boilerplate in six different classes. This is fragile. In fact, the original traits research required stateless traits, (traits provided methods, but no instance data). This turned out to be fragile and research has been done to work around the issues I'm encountering with this limitation.

I think we can resolve this with trusted (protected) methods. The Corinna team has discussed trusted methods a few times and I think it's time to revisit them.

For many programming languages, a trusted method is like a private method which can only be called by subclasses or both other code in the same project. Since Perl doesn't have a well-defined notion of project the latter solution is not good. However, trusted methods being infinitely inheritable means someone can create a subclass of your class and gain access to field $password; This is also not good.

So here's what I'm thinking:

A trusted method, whether provided by an abstract base class or a role, should propagate to the first non-abstract subclass and become a private method in that class. If it's defined directly in a concrete (non-abstract) class, then the first concrete class which inherits it gains it as a private method.

For the field $token above, I think we can define attributes to declare auto-generated methods as trusted or private.

# private to first non-abstract subclass, but all classes in the inheritance
# chain from this class to first non-abstract subclass can call ->token
field $token :reader(:trusted); 

# ->get_password is private to this class only
field $password :reader(get_password:private); # private to this class

As a side benefit of the above, consider my token: it's an array ref. I could convert field $token :reader(:trusted) to this:

field $token;

method token :trusted () {
    return clone($token);
}

The interface remains identical, but now I can use properly coupled state plus behavior to ensure that subclasses can't accidentally break my code.

Of course, this works for roles, too:

role Roke::TokeParser::Token {
    field $token :param :reader(:trusted);
}

class HTML::TokeParse::Corinna::Tag::Start :does(Role::TokeParser::Token) {
    # ->token is private to me and to the role methods
}

Are there other ways of fixing this?

[HaraldJoerg]

Are there other ways of fixing this?

I guess there are. ADJUST phasers to the rescue.

I was a bit startled when reading about how you resolved the issue. By copying the field into six classes, there's nothing left for the base class to do, so there is hardly any reason left for inheritance. Reading the section describing the tokens in HTML::TokeParser confirms: The only thing these concrete classes have in common is "they are initialized from an array reference".

This is a situation I have encountered as well: A constructor parameter which isn't really helpful as an object field.
With the following role the concrete classes can keep the construction parameter around and lazily evaluate the array when needed:

role Role::TokeParser::Token {
    field @token;
    ADJUST (:$token) {
        @token = @$token;
    }
    method token {
        return @token;  # or deep-clone individual elements as desired
    }
}

This has a public reader for the token. It does not pass a reference, so it is safer than having a reader for $token. Having a :trusted reader would add more safety, because you don't need to worry about the references within the token being abused.

There's also an alternative: the classes can pick the relevant values at construction time:

class HTML::TokeParse::Corinna::Tag::Start {
    field $tag;
    field %attr;
    field @attrseq;
    field $text;
    ADJUST (:$token) {
        my $class_indicator = $token->[0]; # if not already stripped by a factory
        $tag = $token->[1]
        %attr = $token->[2]->%*;
        @attrseq = $token->[3]->@*;
        $text = $token->[4];
    }
}

This alternative works without extra modules but does not offer the possibility for a role (or common ancestor) to provide methods which mangle the token array as a whole.

[Ovid]

@HaraldJoerg Since we don't yet have roles, I've gone for the second approach and my class look very similar to what you have, though I've not yet pushed the changes.

For your role, though, we still have the case that I should be able to choose whether I want to expose the token method to the rest of the world. In this case, I very much don't, because that's an implementation detail. In theory, could swap out HTML::TokeParser for another parser, or I could "standardize" the token structure, or do all manner of things that the public doesn't need to know about. For that, I wouldn't want to be forced to maintain arbitrary token structures based on an implementation detail that can change.

Also, keep in mind that for objects, we want the interfaces to be as small as possible. Only give consumers what they need. There's no need for the token since all the working bits are there, so again, I don't want to expose it.

[HaraldJoerg]

I agree to the desire for small interfaces, and I understand the benefit of :trusted in that context!

I also observe that the need for the token method is a consequence of the chosen construction API. I wonder whether the need for this type of non-public functions is mostly relevant during the construction phase ... where the treatment (and interchange) of constructor arguments between different modules are still a bit rough.

[Ovid]

Using $class->new( token => $token ) as a standard interface worked very well because of the dependability of the tokens returns by HTML::TokeParser. Because HTML::TokeParser::Corinna is a factory class returning token classes, even constructtion of the token objects is hidden from the end user. Makes it much easier for them to use and I can safely change construction later, if I need to.

[HaraldJoerg]

Oh yeah, I understand very well. "even construction of the token objects is hidden from the end user." is something which I want to have occasionally. All "modern" Perl OO frameworks enforce a public constructor, so you can't prevent $class->new( token => [qw(whatever I found in the dumpster)] ) from happening.
Unfortunately, the concept of :trusted methods isn't applicable to constructors. A factory is not a subclass of the class(es) it spits out.

[Ovid]

Oh, and you wrote that there's nothing left for the base class to do, but that's not the case. Predicate methods:

class HTML::TokeParser::Corinna::Token :isa(HTML::TokeParser::Corinna::Base) {
    use HTML::TokeParser::Corinna::Policy;

    method is_tag                 {false}
    method is_start_tag           {false}
    method is_end_tag             {false}
    method is_text                {false}
    method is_comment             {false}
    method is_declaration         {false}
    method is_pi                  {false}
    method is_process_instruction {false}
}

But now you might be wondering what HTML::TokeParser::Corinna::Base is. That definitely should be a role, but we don't have roles yet. So my HTML::TokeParser::Corinna token factory and the tokens themselves inherit from this base class:

class HTML::TokeParser::Corinna::Base {
    use HTML::TokeParser::Corinna::Policy;

    method AUTOLOAD {
        our $AUTOLOAD;

        my ( $class, $method ) = ( $AUTOLOAD =~ /^(.*)::(.*)$/ );
        return if $method eq 'DESTROY';
        throw(
            'MethodNotFound',
            method => $method,
            class  => $class,
        );
    }
}

I'm converting to use proper exceptions, so if someone calls a missing method, they'll get an exception object instead of a string (though that's currently a string because overload does not yet work for Corinna).

[HaraldJoerg]

overload does not yet work for Corinna

Innocent, your honor: Exception objects and overloading work in Corinna. Maybe your implementation of throw is to blame?

[Ovid]

@HaraldJoerg See this message I shared about it. Currently, much of the Perl internals cannot handle PVOBJ.

[Ovid]

If you have a working example, I'd love to see it!

[HaraldJoerg]

Quick'n'dirty:

use 5.037;
use feature 'class';
use feature 'try';
no warnings 'experimental';

use Test::More tests => 3;

class Toss {
    method AUTOLOAD {
        our $AUTOLOAD;

        my ( $class, $method ) = ( $AUTOLOAD =~ /^(.*)::(.*)$/ );
        return if $method eq 'DESTROY';
        $self->throw('MethodNotFound',
              method => $method,
              class  => $class);
    }

    method throw ($class,%details) {
        die $class->new(%details);
    }
}

class MethodNotFound {
    field $method :param;
    field $class :param;

    use overload '""' => 'report';
    method report {
        return qq(Method '$method' of class '$class')
    }
}

try {
    Toss->new->oops;
}
catch ($e) {
    is (ref $e,'MethodNotFound',
        "We got an object");
    is ($e->report,q(Method 'oops' of class 'Toss'),
        "The exception reports correct details");
    is ("$e",q(Method 'oops' of class 'Toss'),
        "The exception stringifies correctly");
}

The error message in your report mentions Data::Dumper, and we know that Data::Dumper doesn't handle Corinna objects.

[Ovid]

@HaraldJoerg I'm an idiot. Thank you 😜

[aquanight]

Something I'd like to throw out here: my reading of the proposed solution still has these trusted methods invoked as $self->name. It is my understanding that this means these methods will thus have to still be part of the symbol table. And if they're part of the symbol table, they're effectively public knowledge and little better than public _name with runtime checks.

I would suggest we instead leverage lexical symbols to handle this task. I floated the idea of a :virtual attribute in the IRC channel that would allow classes and roles to declare a field as permitted to be inherited and to in turn inherit/override a field from a superclass or superrole. Mechanically this would essentially make the lexical symbol in the derived class/role be aliased to the corresponding symbol in the base. It does have the limit of allowing the derived class full access to that field, but that can be mitigated by inheriting down private methods (which are essentially SvREADONLYd fields containing method references).