Announcing new principles in the OXID core and how you will benefit from them

Breaking the icy spell of backwards compatibility concerns: In regards to application design, this is a very complex matter, so bear with me while I run you through the story of our concerns and the changes we are now implementing for your benefit.

As you all might know, the OXID Core development team takes care to support you with your projects. For example, backward compatibility is a real concern for us: your projects should be updatable with minimal effort, and setting up projects with your favorite modules should not force you to update your modules again and again.

On the other hand you expect that we keep the shop code up to date. It should use proven and state-of-the-art methodologies and best practices, and you are fully entitled to expect this from us.

Regrettably these goals conflict with each other: if we keep up a strict backward compatibility, major code improvements are simply impossible, and if we try to improve the code base fundamentally, your work flows may break. This is like to dance on a knife’s edge avoiding to fall down on the one or the other side, isn’t it?

This tedious balancing act can be circumvented through the proper application of modern application design. This is the reason why you will notice some fundamental changes in the new release of the OXID eShop compilation (v6.0.2). It’s a number of really small changes for now, but they alter some of the basic principles that have been valid in the past.

The first thing you will notice is that there are private methods in controllers and models. Up to now our strategy has been to allow overwriting everything, all methods have been public or protected. Now for the first time you will find private methods that can’t be overwritten with a module. Let me explain why we changed this behavior.

Most of these private methods were generated when refactoring other methods. Quite a lot of our methods do not follow the single responsibility principle (SRP), so we extracted parts of these methods into smaller ones.

For example, there’s a method deleteUser(). At the beginning of this method there might be some permission checks before the actual deletion code follows. When separating these concerns, we put the permission check into a separate method that might be called checkUserDeletionPermissions() and make it private. Why private? Wouldn’t it be more convenient to make it public, so you may overwrite only this small part in your project if you want to have different checks? On the one hand, you are right: it is better to overwrite a small part than the whole deletion method. But on the other hand, it imposes more code rigidity. From now on, we will always be forced to call checkUserDeletionPermissions() in deleteUser() , because some projects may have overwritten the check method and rely on it being called.

Imagine that we introduce a new permission check strategy, for example using an event mechanism. Your project code will work, but your overwritten check method is not called any longer. You will never notice this, and in your project someone might delete users bypassing all the checks. This is basically the reason why we want to declare such methods as “private”. You got the idea, didn’t you: you’re not loosing any flexibility, because you may overwrite deleteUser() as you used to do it before. Also, your code will still work after any project update, even if we refactored deleteUser() further.

So even if it was tempting to allow overwriting the smaller parts we created, this opportunity may lead you into more trouble than you might imagine. For us this would make the core code even more rigid that it is now, preventing the improvements you rightly expect from us.

What I explained regarding private methods in controllers and models is also valid for the second thing you will notice in the new release. There is a new namespace called Internal, containing some new code for the release. The name is not chosen arbitrary: we literally mean it. Classes in Internal are not overwritable in modules. For now this is just a convention, but in a further release we might even enforce this in oxNew() . Currently there is no code in Internal that you would need to enhance in your projects, so do not worry.

If you look into the Internal namespace, you will find smaller data objects than in the previous core code. You will also find small services implementing interfaces. There is even some rudimentary internal dependency injection mechanism. Currently these are only small steps to use state-of-the-art design techniques in the OXID eShop, hence with future releases we will take more and more this destination.

You don’t have to worry that we want to prevent you step by step from changing the core code. We actually plan the opposite: as the Internal code base is growing, we will provide you with new means (dependency injection, events, …) to alter and extend functionality we introduce there.

There’ll be no loss of the flexibility you have now, because there will still be all the methods available you have now to overwrite them with modules. Additionally, you will get new possibilities to enhance the code without imposing the current rigidity of the core code on us. Currently, this rigidity prevents us from making the necessary changes in the OXID core in a fast and backward compatible manner. The strategy that we follow now will allow us to react much faster on technology changes and market needs without sacrificing any of the flexibility you are used from working with OXID.

So these changes will result in a win-win situation. We will regain flexibility for changes in our code base and you folks will retain the same flexibility as before and will profit from much cleaner ways to adapt the code to your needs.

Should you have questions you’re welcome to use the comment function of this blog. Please stick to English

The concepts discussed in this blog post are well understood and accepted, but private visibility can have subtle side-effects and should be used with care.

The PHP documentation says “… Members declared as private may only be accessed by the class that defines the member.” A quick example with two classes and two methods shows what this means in practice.

  • Class1 has private MethodA and public MethodB
  • Public MethodB calls private MethodA.
  • Class2 extends Class1.
  • Class2 can’t call or replace MethodA.
  • Class2 also can’t modify MethodB.
  • Class2 can only call or replace MethodB.

Now lets look at the example in the blog post, with the public deleteUser() and the private checkUserDeletionPermissions() methods.

Assume I’m happy with the implementation of checkUserDeletionPermissions(), but I need to slightly modify what happens in deleteUser() when checkUserDeletionPermissions() returns true.

I can’t…

My only option is write my own versions of deleteUser() and checkUserDeletionPermissions().

Is that what the author intended by making checkUserDeletionPermissions() private ?

That’s a very good point. I do not see any possibility to call any parent method in this example, because the combination deleteUser() and checkUserDeletionPermissions() is now monolithic again like it was before refactoring, the only way to modify something inside is to overwrite and replace the whole thing with own code, which makes compatibility with other modules impossible, this is not the recommended and clean way. Also, i can override deleteUser(), and the module will still work when oxid changes deleteUser(), that’s true, but also the benefits of the changes made by oxid will be lost. For example, if there is additional data that should be deleted in a new version, it will not be deleted.

@leofonic,

I agree with all your points.

I can see no benefit to partners and developers with the re-introduction of private methods in OXID classes.

Personally, I think it is an unnecessary and retrograde step.

But if there are clear and good reasons for it, I’m happy to be convinced otherwise…

Perhaps OXID Commons presents an opportunity for OXID to explain this in more detail and to get feedback from the community on this important change.

1 Like

+1, unneccessary. For example actionsMain::save() was refactored, which could have been useful when writing a module, but normalizeActionFormData is private.