View Issue Details

IDProjectCategoryView StatusLast Update
15380Bug reports[All Projects] Pluginspublic2019-10-11 22:26
Reporterbismark Assigned To 
PrioritynoneSeverityminor 
Status newResolutionopen 
Product Version 
Target VersionFixed in Version 
Summary15380: Create class or Interface for PluginEvent with names as constants
Description

this helps for IDE-Code Completion

e.g.


interface PluginEventInterface
{
    const EVENT_BEFORE_CONTROLLER_ACTION = 'beforeControllerAction';
}

class PluginEvent implements PluginEventInterface
{
}

//LSYII_Controller::beforeControllerAction
$event = new PluginEvent(PluginEvent::EVENT_BEFORE_CONTROLLER_ACTION);
$event->set('controller', $controller->getId());
$event->set('action', $action->getId());
$event->set('subaction', Yii::app()->request->getParam('sa'));
App()->getPluginManager()->dispatchEvent($event);
TagsNo tags attached.
Complete LimeSurvey version number (& build)develop
I will donate to the project if issue is resolvedNo
Browser
Database & DB-Version10.1.26-MariaDB
Server OS (if known)
Webserver software & version (if known)
PHP Version7.1.8

Activities

bismark

bismark

2019-10-07 16:58

reporter   ~53930

more benefits:

  • event names centralized
  • better documentation
ollehar

ollehar

2019-10-08 14:32

administrator   ~53948

can an interface include constants?

DenisChenu

DenisChenu

2019-10-08 15:28

developer   ~53953

My opinion :

  1. event name must be unique in all LimeSurvey code
  2. event name is used one time only in Plugin

Sometimes i'm unsure add CONSTANT make more readable code : https://github.com/LimeSurvey/LimeSurvey/blob/6cba3e0c964df2cc8430e103b0fa6baec3e97480/application/helpers/questionHelper.php#L71

Auto documentation with such solution can not include input and output
Then no real reason for this.

dschirge

dschirge

2019-10-09 14:15

reporter   ~53973

I agree with Bismark, it would be very useful for future development.
I would implement it this way:

use MabeEnum\Enum;
/**

  • @method Events BEFORE_CONTROLLER_ACTION()
    **/
    class Events extends Enum
    {
    // Enumerators will be generated from public constants only
    public const BEFORE_CONTROLLER_ACTION = 'beforeControllerAction';
    }

additionally benefits:

  • you can compare event types internal if necessary and even constants with the same value are different because of the Enum implementation.

  • type safety

  • the already mentioned benefits

Example:
public const EV_A = 'a';
puclic const EV_B = 'a';

Events:: EV_A()->is(Events::EV_B()); //false
It is because if the is() implementation which does a comparisment of the value and the name of the enumerator.

Mazi

Mazi

2019-10-09 15:03

partner   ~53974

@c_schmitz, can you comment on this? For us - and other plugin developers - it would be pretty useful and I am rather sure it will not have any side effects when adding this to the core.

dschirge

dschirge

2019-10-11 22:26

reporter   ~54013

I created an example and uploaded to a fork in GitHub.
You can loot at this in https://github.com/pop1989bb/LimeSurvey/commit/b516890335d66f0ec295ba7f6cc4614b621a9b9e
You coud use the new system via $this->subscribe(GlobalEvents::AFTER_FIND_SURVEY()->getValue());
The function "getValue()" returns the string I assigned to the enum.
I suggest to use it this way for compatibility reasons. In a future version (maybe v4 or v5) I advise to enforce using the enums. It will reduce errors.
I didn't implement all events nor made comments for all of the already declared events. I would do this if my suggestion is accepted.

Issue History

Date Modified Username Field Change
2019-10-07 16:07 bismark New Issue
2019-10-07 16:58 bismark Note Added: 53930
2019-10-08 14:32 ollehar Note Added: 53948
2019-10-08 15:28 DenisChenu Note Added: 53953
2019-10-09 14:15 dschirge Note Added: 53973
2019-10-09 15:03 Mazi Note Added: 53974
2019-10-11 22:26 dschirge Note Added: 54013