Using a service based on ID of record requested

[NotionCommotion]

My database schema has a supertype charts table and subtype tables for bar charts, pie charts, gauge charts, and line charts.  Ajax requests are received just with a public ID which identifies a given chart.  Based on the type of chart corresponding to the received ID, a different service is used.  Something like the following will work, but is horribly ugly.  I feel I should be either determining the specific type and thus the service is either some sort of factory/controller stored in the container, but I am not sure how to give it the ID.  Any recommendations?  Thank you

<?php

function getChartService($type, $pdo, $properties) {
    switch($type) {
        case 'bar':
            return new Chart(new BarChart(), $pdo, $properties);    //And maybe add other objects as necessary
            break;
        case 'pie':
            return new Chart(new PieChart(), $pdo, $properties);    //And maybe add other objects as necessary
            break;
        case 'gauge':
            return new Chart(new GaugeChart(), $pdo, $properties);    //And maybe add other objects as necessary
            break;
        case 'line':
            return new Chart(new LineChart(), $pdo, $properties);    //And maybe add other objects as necessary
            break;
        default: throw new Exception('Invalid service');
    }
}

$app = new \Slim\App();

//I suspect I should be doing this
$container = $app->getContainer();
$container['ChartFactory'] = function($c) {
    //But how do I access the $args['publicId'] value? 
};

$app->get('/charts/{publicId}', function (Request $request, Response $response) {
    // query only the supertype table using publicId to determine what type of chart it is
    $stmt=$this->pdo->prepare('SELECT id AS idDatabasePK, type, name, etc, idPublic FROM charts WHERE idPublic=? AND accountsId=?'); //Or maybe get rid of "type", and determine it based on the subtype joined table
    $stmt->execute([$args['publicId'], $this->account->id]);    //account->id set in middle-ware based on key sent in header
    if(!$properties=$stmt->fetch()) throw new Exception('Invalid public ID');
    $chartService=getChartService($rs->type, $this->pdo, $properties);
    $rs=$chartService->read($args['idDatabasePK']);
    return $response->withJson($rs, 200);
});
$app->delete('/charts/{publicId}', function (Request $request, Response $response) {
    // similar to get request
});
$app->put('/charts/{publicId}', function (Request $request, Response $response) {
    // similar to get request
});

$app->post(_VER_.'/charts/{type: bar|pie|gauge|line}', function (Request $request, Response $response, $args) {
    // Get object based on user requested chart type
    $chartService=getChartService($args['type'], $this->pdo, []);
    $rs=$chartService->create($request->getParsedBody());
    return $response->withJson($rs, 200);
});

$app->run();

[requinix]

At some point you'll have to map the chart string to a chart class. Sure, I would make it a more standard factory pattern by putting it in the base Chart class, but it will still be using your switch($type) code. If you really don't like the switch then you can

public static function /* Chart:: */factory($type, $pdo, $properties) {
	$class = /* __NAMESPACE__ . "\\" . */ ucfirst($type) . "Chart";
	if (ctype_alpha($class) && class_exists($class)) {
		return new self(new $class(), $pdo, $properties);
	} else {
		return null;
	}
}

[NotionCommotion]

Sure, I would make it a more standard factory pattern by putting it in the base Chart class

 

I don't really have any issues with the switch statement (or either using a hardcoded array to implement the map).  Yeah, both go against all the OOP dogma which I am trying to embrace, but both are readable and will never be reused which I feel makes their use okay.

 

But how to implement "a more standard factory pattern" for this scenario has got me stalled.  How do I invoke the "base Chart class" before I know what to inject (or extend)?   Do you mind providing an example (pseudo code is okay if you wish).

 

Thank you

 

By the way, interesting slumbermancer quote

[requinix]

Well, the fact that Chart uses child Chart classes is a bit odd. It should be a matter of just one of them.

return new BarChart($pdo, $properties);

But the factory could be as simple as a static method on Chart. Like the one I posted, but with the original switch or whatever. Then

function getChartService($type, $pdo, $properties) {
	$chart = Chart::factory($type, $pdo, $properties);
	if ($chart) {
		return $chart;
	} else {
		throw new Exception('Invalid service');
	}
    }
}

[NotionCommotion]

Well, the fact that Chart uses child Chart classes is a bit odd. It should be a matter of just one of them.

 

This is just my attempt to inject Chart with an object (i.e. BarChart or some other chart type) instead of using inheritance.  I am obviously doing something wrong as you did not even suspect that I was trying to do so.  Originally, I had BarChart extend Chart, but eventually it became a complicated mess.  Please comment.

 

But the factory could be as simple as a static method on Chart. Like the one I posted, but with the original switch or whatever.

 

I am okay with the factory part.  In fact, the factory is already implemented by using Pimple\Container\offsetGet().

 

Please take a step back.  How do I get publicId to the factory?  I think I was just being stupid.  Should I just be using getChartObject() as shown below?

 

$container = $app->getContainer();
$container['ChartFactory'] = function($c) {
    //But how do I access the $args['publicId'] value? 
};

$app->get('/charts/{publicId}', function (Request $request, Response $response) {
    // All I know at this time is publicId
    // $this->ChartFactory references $container['ChartFactory']
    // and ChartFactory needs to be given publicId in order to identify the create object.
    // How do I pass this value to it?

    // Wait a second.  ChartFactory doesn't have a constructor for me to pass publicId to.  Should I just do...
    $chartService=$this->ChartFactory->getChartObjectById($args['publicId']);
    $rs=$chartService->read($args['publicId']);
    return $rs->withJson($rs, 200);
});

 

One other aspect which has got me confused is what to do with $properties.  These properties are a byproduct when the specific chart type was derived by querying the database using idPublic, and includes the chart name, the chart's primary key (which isn't exposed to the user) and other properties, and are part of a chart entity and I don't think belong in a service.  Agree?  My original intent was to save a query by using them, but I am concerned that doing to might bring too much complication.  I suppose I can save them in ChartService ($this->chartProperties) when they are sent to its constructor, and then when a method is executed on ChartService, grab the properties and make them available to the mapper...

[requinix]

This is just my attempt to inject Chart with an object (i.e. BarChart or some other chart type) instead of using inheritance.  I am obviously doing something wrong as you did not even suspect that I was trying to do so.  Originally, I had BarChart extend Chart, but eventually it became a complicated mess.  Please comment.

I got that there was injection, but what I don't get is why you were doing that instead of inheritance. If you were injecting a BarChart into a, I don't know, graphics processing class, that's totally fine, but injecting a class that represents a type of chart into a class that represents charts as a whole is what confuses me. Sure, you could do it that way, but why when inheritance would be easier?

 

Supposedly easier. What sort of mess?

 

I am okay with the factory part.  In fact, the factory is already implemented by using Pimple\Container\offsetGet().

 

Please take a step back.  How do I get publicId to the factory?  I think I was just being stupid.  Should I just be using getChartObject() as shown below?

Yes. But remember that the function added to the container is just to get the ->ChartFactory. If you want to do ->ChartFactory->getChartObjectById then the function returns some class with a getChartObjectById method. The function can't get the publicId but that's okay because it doesn't need to.

 

Really, saying that you're using Pimple to "implement" the factory is incorrect. You're using it to get access to thing (an object instance) which you use as a factory. For all it matters that factory could instead be stuffed into a global variable - the way you use it is the same: get the instance and call method on it.

 

One other aspect which has got me confused is what to do with $properties.  These properties are a byproduct when the specific chart type was derived by querying the database using idPublic, and includes the chart name, the chart's primary key (which isn't exposed to the user) and other properties, and are part of a chart entity and I don't think belong in a service.  Agree?  My original intent was to save a query by using them, but I am concerned that doing to might bring too much complication.  I suppose I can save them in ChartService ($this->chartProperties) when they are sent to its constructor, and then when a method is executed on ChartService, grab the properties and make them available to the mapper...

The "arbitrary metadata required by a factory-ed class" problem can be solved a bunch of ways. Storing the metadata (your "properties") separate from the class type and just knowing that every child class can have an optional array of stuff is one way.

 

Whether that knowledge goes into the service is another question. It sounds to me like a job for a chart database model, as in a database model representing the charts. If the service knows that it's dealing with a chart then it uses the chart model to load data, then magically that model turns into a real class however you want, eg. by passing it to a factory method as in Chart::factory($chart_model_instance). That way the details about how the chart is represented in the database has an authoritative class to handle it.

 

I can't follow along with the assorted "services" and whatnot, it's all too confusing to me, but given the above I think it would go:

1. Get chart model according to the publicId

2. Pass chart model to chart factory and receive chart

3. Pass chart to chart service factory and receive chart service?

4. withJson and whatever

[NotionCommotion]

Thank you requinix for your comprehensive reply.

 

Injection versus inheritance

 

Agree that injecting a class that represents a chart into another class that represents a chart is a bit off.  Only did so in an attempt to be more injection minded, and will go back to inheritance.

 

The mess you ask?  A excessively big class which had this multi-dimensional method to chart type aspect to it.  I don't think it was caused from using inheritance, but from using inheritance incorrectly.  I think I need to break the class into smaller classes so that inheritance does not bring unwanted baggage, and then inject those smaller class as applicable into a chart. 

 

A little background.  There are four main types of charts and each has multiple sub-types which act identically.  An important consideration is that all chart types have series, only some types have categories, and data is associated with these series and charts differently based on the chart type.  The following describes each type and lists some of their properties.

 

• Normal charts (bar, column, area, etc):
  • ID, PublicId, name, color palette, etc.
  • Multiple series and categories array where each has a name and specified position, and the intersection of a series and categories has a value.
• Pie charts (pie, donut, etc):
  • ID, PublicId, name, color palette, etc.
  • Multiple series where each has a name and specified position as well as multiple categories, and these categories (typically only one category) each have a name, specified position, and value.
• Gauge charts (solid, dial, etc):
  • ID, PublicId, name, color palette, etc.
  • Multiple series (typically only one series) where each has a name, specified position and value.
• Time charts (with several flavors):
  • ID, PublicId, name, color palette, etc, as well as time duration and time interval.
  • Multiple series where each element has a name, specified position, time offset and value.

 

Many of the methods are shared between all of them, but not allows.  Below is a partial list of required methods:

• Get all charts (ID, name, type, and other common attributes).
• Get chart by ID along with its series and categories (as applicable for given chart type)
• Create new chart of given type.
• Create new gauge chart with a single series with a value.
• Delete chart by ID.
• Edit chart's primary attributes such as name and color palette as well as chart type specific attributes.
• Edit chart's series and categories attributes such as name as well as specific attributes for each chart type.
• Reorder chart series and categories (gauge and time charts do not have categories).
• Add chart series and categories as applicable for each chart type.
• Delete chart series and categories.
• Clone a given chart given its ID and give it a new name.
• Generate Highcharts JSON to create a given chart by ID.

The smaller classes I referred to above will be the series and categories.  Then, I need to use inheritance if necessary to extend series as well as categories, use inheritance if necessary to extend chart, and then inject the series and categories into the chart.

 

I am still uncertain whether these smaller classes just apply to the entity classes or will also be needed for the service and mapper classes.  More on this on the next topic.

 

Factory Discussion  Understood.  Thanks.

 

 

Whether that knowledge goes into the service is another question. It sounds to me like a job for a chart database model, as in a database model representing the charts. If the service knows that it's dealing with a chart then it uses the chart model to load data, then magically that model turns into a real class however you want, eg. by passing it to a factory method as in Chart::factory($chart_model_instance). That way the details about how the chart is represented in the database has an authoritative class to handle it.

I can't follow along with the assorted "services" and whatnot, it's all too confusing to me, but given the above I think it would go:
1. Get chart model according to the publicId
2. Pass chart model to chart factory and receive chart
3. Pass chart to chart service factory and receive chart service?
4. withJson and whatever

 

I've been dwelling on this topic as well.

1. Is the chart model just data provided by the DB stored in standard classes and arrays?
2. Does the chart factory provide a "chart" object that can interact with the database?  I noticed you have not used the phrase "mapper".  Does it not exist in this scenario and instead functionality is performed by this chart object?
3. Since different chart types have different methods and requirements, I suppose a service factory makes sense.  I was thinking to make this service object have public sub-methods such $chartService->series[0]->update('name', 'Total Usage'); which can be accessed by the app.
4. and whatever...

 

Thanks again for sharing your expertise!

[requinix]

I think I need to break the class into smaller classes so that inheritance does not bring unwanted baggage, and then inject those smaller class as applicable into a chart.

Sounds good.

 

There are four main types of charts and each has multiple sub-types which act identically...

Sub-class the hell out of it. Really. Try starting off with getting a handful of drastically different chart types working independently, even by copying and pasting between classes, then examining the final code to see where you can refactor commonly recurring parts into Chart or some other additional parent class(es).

 

Speaking of, consider Chart being a somewhat minimal class and then adding functionality through interface-esque classes. For example, a CategorizedChart for the types that use categories, so that behavior isn't in Chart but also not forced into each child that needs it.

 

Traits could be particularly useful here too. If it makes more sense to think of a particular chart aspect as a feature of the chart rather than as a natural part of what the chart represents then a trait would be better than inheritance. Especially if said feature can show up in different places in the chart class hierarchy without any clear rules about when or where it's inherited from a parent.

 

• Is the chart model just data provided by the DB stored in standard classes and arrays?
• Does the chart factory provide a "chart" object that can interact with the database?  I noticed you have not used the phrase "mapper".  Does it not exist in this scenario and instead functionality is performed by this chart object?
• Since different chart types have different methods and requirements, I suppose a service factory makes sense.  I was thinking to make this service object have public sub-methods such $chartService->series[0]->update('name', 'Total Usage'); which can be accessed by the app.
• and whatever...

• Not necessarily "just", but the main focus would be the data and its representation in the database. Like the properties could be stored in multiple ways, like secondary tables or a JSON string, but this class would make it accessible in code using one standardized form (eg, a $properties array). It's not a big deal but does take care of that bit of complexity so nothing else needs to know.
• If by "mapper" you mean something that understands the database structure and can translate that into and out of a class, as in a way of moving beyond the traditional style of manually querying a database, reading results, and manipulating objects, then that's an implementation detail. The chart model would be the mapped class and the mapper would be the mechanism used to move data between it and the database. The mapper would handle lots of the work needed for a database model so it would be acceptable to bypass the "chart model" and go straight to the actual Chart classes. The problem is that the Charts are so variable that it might be difficult for the mapper to do so; in that case you would keep the simplistic chart model, use the mapper as the middleman between the model and database, and the model would be responsible for the remaining work of serializing and unserializing Chart instances (even if the actual factory-type code used to do so lives in Chart).
• Sounds good. Though instead of a member variable I make it methods (getSeries($i) and maybe setSeries($i,???)) so that calling code can't manipulate the series information incorrectly.
• I think I might be beginning to finally understand this "service" thing: you seem to use the term in places where I would not, but the addition doesn't seem to change the meaning beyond what I initially understand. Like, I think of this as dealing with chart classes and factories whereas you call them chart services and service factories, and it seems we're still talking about the same concepts. So that's nice.

[NotionCommotion]

Sub-class the hell out of it. Really.

It recently sunk in that I needed to sub-class but based on your comment will do so more zealously.

 

I initially got all working (kind of) with six classes (Chart, CategorizedChart, and the four specific primary chart types) but as you can imagine it is complicated and prone to error. Following is a critique of how well I had applied my newly learned SOLID principles:

 

S - Single-responsibility principle. Not even close.

O - Open-closed principle. Yes for the app's use of a chart, but not at all for the chart internals.

L - Liskov substitution principle. Yes for the app's use of a chart, but not at all for the chart internals.

I - Interface segregation principle. I have multiple methods which needed to be implemented just to respond "this method is not implemented for this chart type".

D - Dependency Inversion Principle. Wasn't even thinking this way.

 

Speaking of, consider Chart being a somewhat minimal class and then adding functionality through interface-esque classes. For example, a CategorizedChart for the types that use categories, so that behavior isn't in Chart but also not forced into each child that needs it.

I not sure I understand. My intention is to remove as much functionality as possible from the chart classes and inject it, and use inheritance for the chart classes as shown.

• CommonChart (this is my term for bar, area, etc) and PieChart extends Chart extends CategorizedChart
• GaugeChart and TimeChart extends Chart

Does it sound that I am following you? Note that I will not implement a method for unsupported methods (or instance, delete a category from a gauge chart where gauge charts do not have categories) but will respond using __call(), and will likely implement a method to list supported methods using get_class_methods().

 

Traits could be particularly useful here too.

Thank you, I will keep them in mind.

 

3. Sounds good. Though instead of a member variable I make it methods (getSeries($i) and maybe setSeries($i,???)) so that calling code can't manipulate the series information incorrectly.

Yes, agree. While $chartService->series[0]->update('name', 'Total Usage'); would be kind of cool in a JavaScript sort of way, it will be difficult to prevent the ability to to incorrectly manipulate the series info, and planned on changing to as what you show.

 

2. If by "mapper" you mean something that....

4. I think I might be beginning to finally understand this "service" thing....

ignace totally sold me on using a service based on How best to implement slim with OOP and I've been trying to identically mimic at all costs Slim action-domain-responder with regards to the service, mapper and entity classes. While I think this is often a great approach, I also think I am trying to apply them inappropriately or to where they are not applicable.

 

Some functionality such as deleting a chart is consistent for all chart types since the database will cascade upon delete, and can use the mapper concept as described in the Slim tutorial. Creating a new gauge and adding a series to it all in one transaction, however, is much more involved and I better first "build" the chart object, and then save it in the DB as one transaction.

 

Another aspect which differs from the tutorial is how my application requires the DB to first be queried to determine the chart type. The "service" concept implies that no previous querying need be required, so I must either throw away the data from this previous query or kind of think differently about things, and I think I need to do the later.

 

In hindsight, I definitely see how my use of various terms brought you some confusion. So, if staring over and forgetting all my misused terms, what would you recommend calling the main concepts so I may start using those definitions going forward?

 

Thank again!