[PROPOSAL] ilCtrl::setCmd() and ilCtrl::setCmdClass() deprecation.

[thibsy]

Hi all,

The PR #4962 and #4772 drew my attention to the deprecation of ilCtrl::setCmd and ilCtrl::setCmdClass again. With this PR I want to start a discussion of whether these methods should be deprecated or not.

Why do these methods exist?

setCmdClass is used to dynamically add/set a new command class, if the request doesn't "reach far enough". What I mean by this, is that there are requests which contain a certain command-class as a target. Once the target is reached by forwarding through each class(-node), it might be possible that further GUI classes should be called. An example for this would be the learning-progress:

Object-GUIs like e.g. ilObjCourseGUI create link-targets with ilCtrl to reach the ilLearningProgressGUI. The ilLearningProgressGUI is not the final target though, because this class (as to my understanding) only serves as a dispatcher, that calls specific LP-GUIs according to internal settings. Now because the ilObjCourseGUI doesn't want to make these checks it only creates a link-target to this dispatcher. The ilLearningProgressGUI will then use ilCtrl::forwardCommand to delegate to the next LP-GUI. If this new GUI then tries to generate link-targets to itself again, ilCtrl won't be able to find a valid path to it, because ilCtrl's current command class is still ilLearningProgressGUI. Therefore, the dispatcher uses ilCtrl::setCmdClass to tell ilCtrl the new location is the new LP-GUI.

setCmd on the other hand is used to trick ilCtrl into returning the last set command instead of the requested one.

Why should these methods be removed?

Now the problem with that is, by using setCmd and setCmdClass you implicitly change the control-flow and output of getLinkTarget, getNextClass and getCmd. IMO this leads to pretty unpredictable code as well, because you can never know what previous classes might have done to trick ilCtrl into reaching the current command-class. Dynamically adjusting the control-flow also makes it extremely difficult to move into a direction where static routes are possible (and I believe we'd all like that very much). I also think it's not quite transparent if the command-class and command of the request differ from the one's delivered by ilCtrl. If SomeGUI is called with cmd=foo&cmdClass=SomeOtherGUI I begin to wonder how I might react on retrieving bar from getCmd and SomeOtherGUI from getCmdClass. How should SomeGUI know how to react? should it react at all?

How can these method-calls be replaced?

From an ilCtrl POV, using setCmdClass is just a comfortable workaround for properly using the methods getLinkTarget and getLinkTargetByClass. The problem, as I tried to explain with the LP example, could be solved by telling ilCtrl the proper way to a target-class by using an array: ilCtrl::getLinkTargetByClass([BaseClass, MaybeAnotherGUI, CommandClass]). It is possible, that CommandClass could be called via two different base-classes, but this scenario is covered by ilCtl, because you can just leave out the BaseClass in this array and ilCtrl will use the one of the current context. Same goes for MaybeAnotherGUI.

setCmd on the other hand is a total abomination, which basically just overrides the requested command. I believe we can all agree on why this is bad. I would therefore recommend to not even replace it.

Why does it need to be discussed?

Since using setCmdClass has become sort of a feature of ilCtrl and has many usages, I wanted to discuss whether this method can be dropped or not. As a maintainer, I strongly recommend to remove these methods for the reasons I've stated, but because this is a rather central service that affects many other maintainers I choose to discuss this again. Dropping these methods would mean that every getLinkTarget and getLinkTargetByClass call might break, and it would only be discovered at runtime.

If you know about any edge-cases where setCmdClass cannot be "replaced" by proper link-target generations, and/or have questions or comments, please let me know!

Kind regards!