Contravariance with PHP7.4

[NotionCommotion]

I have an interface with method: public function foo(string $arg1, string $arg2);

With PHP7.4, I can have a class relax the type without errors such as Dog::foo(string $arg1, $arg2) ($arg2 is not required to be a string)

I incorrectly thought I could even relax the constraint more such as Dog::foo(string $arg1){} and not even pass $arg2 because extra arguments which exceed a method's placeholders are ignored without error but I received error:  Declaration of Dog::foo(string $arg1) must be compatible with AnimalInterface::foo(string $arg1, string $arg2).

Are my only options to include $arg2=null in the relaxed methods and label the methods something $dummy1, $dummy2..., or pass them in an array (which I don't wish to do)?

Is passing extra parameters to the same method as I am doing considered bad practice, and if so what is considered the correct way?

Thanks

 

<?php
declare(strict_types=1);

ini_set('display_errors', '1');

echo "PHP Version: ".PHP_VERSION.PHP_EOL;   // => PHP Version: 7.4.1

interface AnimalInterface
{
    public function foo(string $arg1, string $arg2);
}

abstract class Animal implements AnimalInterface
{
    protected string $name;

    public function __construct(string $name)
    {
        $this->name = $name;
    }

    protected function test($method, $arg1, $arg2)
    {
        echo $method.' '.$this->name.' '.gettype($arg1).' '.gettype($arg2).PHP_EOL;
    }
}

class Cat extends Animal
{
    public function foo(string $arg1, string $arg2)
    {
        $this->test(__FUNCTION__, $arg1, $arg2);
    }

    public function bar(string $arg1, string $arg2)
    {
        $this->test(__FUNCTION__, $arg1, $arg2);
    }
}

class Dog extends Animal
{

    //public function foo(string $arg1){}         //generates an error

    public function foo(string $arg1, $arg2=null)
    {
        $this->test(__FUNCTION__, $arg1, $arg2);
    }

    public function bar(string $arg1)
    {
        $arg2=$arg2??null;
        $this->test(__FUNCTION__, $arg1, $arg2);
    }
}

$kitty = new Cat("Ricky");
$doggy = new Dog("Mavrick");

$kitty->foo('arg1', 'arg2');
$doggy->foo('arg1', [1,2,3]);

//$kitty->bar('arg1', (int)222);  //will error using strict mode only
$doggy->bar('arg1', (int)222);

$kitty->bar('arg1', 'arg2');
$doggy->bar('arg1', [1,2,3]);

Quote

PHP Version: 7.4.1
foo Ricky string string
foo Mavrick string array
bar Mavrick string NULL
bar Ricky string string
bar Mavrick string NULL

 

[requinix]

5 minutes ago, NotionCommotion said:

With PHP7.4, I can have a class relax the type without errors such as Dog::foo(string $arg1, $arg2) ($arg2 is not required to be a string)

I incorrectly thought I could even relax the constraint more such as Dog::foo(string $arg1){} and not even pass $arg2 because extra arguments which exceed a method's placeholders are ignored without error but I received error:  Declaration of Dog::foo(string $arg1) must be compatible with AnimalInterface::foo(string $arg1, string $arg2).

Thinking about it as "relaxing" will lead you into conclusions like that.

Contravariance is about making sure that if your class claims to be some Thing, it is capable of supporting at least as much as whatever the Thing could support.

AnimalInterface says that it supports calling foo with a string argument #1 and a string argument #2.

$animal_interface->foo($string_1, $string_2);

If your Dog removes the type from argument #2 then that's okay because it still supports a string - it's just that it supports other types as well.

$animal_interface = new Dog();
$animal_interface->foo($string_1, $string_2); // still works

If your Dog removes argument #2 entirely then that's not okay because you no longer support that argument. Yes, PHP allows passing more arguments than required, and you could let you get at it through func_get_arg/s, however (a) that's only because of how PHP itself supports function calling, but more importantly (b) if your function doesn't care about argument #2 then there might be something wrong with it.

5 minutes ago, NotionCommotion said:

Are my only options to include $arg2=null in the relaxed methods and label the methods something $dummy1, $dummy2..., or pass them in an array (which I don't wish to do)?

Is passing extra parameters to the same method as I am doing considered bad practice, and if so what is considered the correct way?

Your example is too abstract to answer this effectively. What's the real situation?

[NotionCommotion]

13 hours ago, requinix said:

Thinking about it as "relaxing" will lead you into conclusions like that.
... it is capable of supporting at least as much as whatever the Thing could support.
... it's just that it supports other types as well.

Thank you.  Well said and sunk home.  Contravariance (and covariance) relaxes the interface precision, but solely for the class's customer application's benefit and not the class itself, and the given class actually has to fully implement the status quo and then do more.  While I technically knew this was the intent, I wasn't really thinking this way.  Am I on track?

13 hours ago, requinix said:

Your example is too abstract to answer this effectively. What's the real situation?

Agree, it is too abstract.  Back to dealing with displaying real time and historical data in some sort of chart format and refactoring how data is retrieved in order to popular a given chart. 

Every chart has an ID (which is used to request the chart), name, title, etc as well as one or more series (let's focus on the series).  Each series will have a name and other associated data as well as multiple data nodes where each data node has a value, name, etc.

There are different types of charts which have different data requirements:

1. Line Chart:  The series has sequence of data nodes and all series for a given chart have the same number of data nodes.
2. Categorical Chart: Uses an X/Y format where a common sequence of categories is associated with each series and all series for a given chart have the same number of data nodes.
3. Pie Chart: Each series for a given chart can have different number of data nodes.
4. Gauge Chart: Each series only has one data node.

There are two basic ways values for the charts are generated:

1. Each series has multiple data nodes where each's value is based on a given physical parameter current values or a given physical parameter aggregate values of historic data (mean, max, etc).
2. Each series is only associated with a single parameter, but multiple data nodes are obtained by grouping the parameter on a given time interval.  There are also different derivatives of these such as grouping based on a sample size which is applicable for a line chart, or on either a given time duration (seconds in a day, week, month, etc) or on a time unit (whole days, weeks, months, etc) which is applicable for the different chart types.

The database schema utilizes a base chart table with one-to-one relations to a subtable for each chart type.  Multiple charts are typically presented simultaneously, and I am querying the base table with left joins to subtables (better performance, but granted does not obey open-closed principle), and handling the aggregate values of historic data as well as the group by time requirements elsewhere with a different query.

Each chart type class extends an abstract Chart class and each chart type series will extend an abstract ChartSeries class or maybe just implement an interface.

So, I query the database, get a record which defines a given chart's type and associated meta data and create a chart object.  I also get multiple records which define the various chart series and send this data to the applicable recently created chart object's addSeries() method, and similarly get additional records which define the various series data nodes.  Each record from the query provides data for the chart, series, and data node, but I just skip creating a chart or series if it was created using a previous record.

For a pie chart, I just get the chart ID, name, JSON options data, timezone, etc and pass it to a PieChart's constructor to create the chart object, then get just the series ID and name and call $series=$chart->addSeries(int $id, string $name), and then get the data node data such as ID, label, and data associated with the physical parameter and send it to $series->addDataNode(int $id, string $label, Parameter $parameter).

But for a group by time chart, I also get time meta data such as the total chart's time duration and feed it to a TimeChart's constructor to create the chart object.  Then for each series, I also get additional time meta data such time offset in the past and call $series=$chart::addSeries(int $id, string $name, TimeObject $timeOffset), and then the same as for a pie chart call $series->addDataNode(int $id, string $label, Parameter $parameter).  Note that the database will only return a single data node record for these types of charts and I've considered not implementing TimeChart::addDataNode() and instead including this information in TimeChart::addSeries(), but I feel it is cleaner being consistent with the other chart types (agree?).

So, for all chart types except time grouping charts, I have method addSeries(int $id, string $name), but for time grouping charts I need method addSeries(int $id, string $name, TimeObject $timeOffset).  There are also some other minor differences between different chart types, but I feel if I get this right, the others will fall into line.

Maybe I am still not thinking correctly and appreciate any advice you are willing to provide.

Thanks

[requinix]

The common denominator is that the method takes an ID, name or label, and extra data. For regular charts there is no extra data (so far...), for time charts there is some time thing, and for pie charts there is some data thing.

Or alternatively, if there is any way you could modify the pie and time charts to work without the data/time thing, the common denominator is an ID and name/label. You could then use a new interface that provides an optional third argument according to whatever - optional so it remains compatible with the base interface.

[NotionCommotion]

14 hours ago, requinix said:

The common denominator is that the method takes an ID, name or label, and extra data. For regular charts there is no extra data (so far...), for time charts there is some time thing, and for pie charts there is some data thing.

Yes, key phrase is "so far".  This seems to be a common occurrence for me.  I go down some path and then later find I need some other data for a given implementation and need to go back and change my interfaces which is rather a pain.

14 hours ago, requinix said:

Or alternatively, if there is any way you could modify the pie and time charts to work without the data/time thing, the common denominator is an ID and name/label. You could then use a new interface that provides an optional third argument according to whatever - optional so it remains compatible with the base interface.

Are you suggesting some grab bag which anything extra can go in or something else?  I don't think so since doing so wouldn't require the pie and time charts to work with their extra things.

interface ChartInterface
{
    public function addSeries(int $id, string $name, ?OtherStuffInterface $otherStuff);
}

 

[requinix]

2 hours ago, NotionCommotion said:

Yes, key phrase is "so far".  This seems to be a common occurrence for me.  I go down some path and then later find I need some other data for a given implementation and need to go back and change my interfaces which is rather a pain.

Obligatory "welcome to the world of software development".

Things change. Try to anticipate and plan for what might happen, but otherwise you're kinda stuck just responding to changing business needs.

Saying that everything gets an optional data structure of "stuff" makes sense. Not all chart types may need it, but a general purpose "stuff" thing doesn't sound unreasonable to me.

2 hours ago, NotionCommotion said:

Are you suggesting some grab bag which anything extra can go in or something else?  I don't think so since doing so wouldn't require the pie and time charts to work with their extra things.

interface ChartInterface
{
    public function addSeries(int $id, string $name, ?OtherStuffInterface $otherStuff);
}

 

It could only work if the pie and time charts could come up with some sort of reasonable default behavior. If not, then...

[NotionCommotion]

4 minutes ago, requinix said:

It could only work if the pie and time charts could come up with some sort of reasonable default behavior. If not, then...

I expect they will not have some sort of reasonable default behavior.

If not, then?  The answer to the universe?  World peace?

I was thinking of adding some OtherStuffInterface::getValue(string $name) method but I don't think doing so is really right.  Alternatively, I can create interfaces OtherTimeChartStuffInterface and OtherPieChartStuffInterface which extend OtherStuffInterface, but not sure if this really makes sense.

I expect you have a viable answer but want me to think it through.  If so, mind given another clue?

[requinix]

2 hours ago, NotionCommotion said:

If not, then?  The answer to the universe?  World peace?

If not, then you can't use that approach of the root interface having two arguments and child interfaces for pie and time charts having their own interfaces with three arguments.

2 hours ago, NotionCommotion said:

I was thinking of adding some OtherStuffInterface::getValue(string $name) method but I don't think doing so is really right.  Alternatively, I can create interfaces OtherTimeChartStuffInterface and OtherPieChartStuffInterface which extend OtherStuffInterface, but not sure if this really makes sense.

I expect you have a viable answer but want me to think it through.  If so, mind given another clue?

Eh, the only thing I have is to create a sort of ExtraChartDataThing class, then you have methods for the pie and time charts to add data to it, then those classes can pull the data back out when they want it. And if there is no data they throw up.

Or slight variation, maybe better, it's still a key/value store, but the key is a class name (thus unique) and the value an instance of it, then you stuff your Parameter/TimeOffset data into without needing special chart methods.