Skip to content

Conversation

@alex40724
Copy link

@alex40724 alex40724 commented Jun 18, 2019

@alex40724 alex40724 requested a review from mjansenDatabay June 19, 2019 11:27
*
* @author killing@leifos.de
*/
interface Parameters

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a friend of empty interfaces just for the purpose of having created such :-). But of course this is an approach/prototyp and maybe we need this kind of interface later.

*
* @author killing@leifos.de
*/
class API

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implements \ILIAS\API\Int\API?

*
* @author killing@leifos.de
*/
class FactoryCollection implements I\FactoryCollection
Copy link

@mjansenDatabay mjansenDatabay Jul 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of this factory stack created by the construction order of the factories, to determine parent/child relations when dispatching the command later on.

Maybe this should be renamed to FactoryCreationStack or something similar.

* @param int $actor_id
* @throws Exc\PolicyFailed
*/
public function dispatch(Int\Command $command, int $actor_id)
Copy link

@mjansenDatabay mjansenDatabay Jul 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When first reading this I had some pain to understand this, but know I got this!

I like this approach which is similar to ilCtrl/executeCommand/forwardCommand, so a handler could be used by different parent handlers (course AND group).

*/
public function handle(API\Int\Command $command, int $actor_id)
{
if ($command instanceof AddCommand)
Copy link

@mjansenDatabay mjansenDatabay Jul 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll most propably need to know something about the parent(s) (or certain information of this/them) in the current execution chain here. In the concrete case we maybe need to distinguish a course parent from group parent.

Update: Okay, AFAIK you already addressed this in the README.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants