4

Not sure whether I picked the proper title. But anyway.

I've got to support and to develop already existing project.

It's built on OOP.

I've got such models as Order, Product. They are used very often into code.

Every model is bound to a corresponding table.

Client wants to integrate a new kind of customers into system. This kind of customers has different data associated with them and order completely different kinds of Products. So Order will look different.

What I decided is to not to mix the old kind of Order and the newly kind of it. At first I put them into separated tables in order not to break the working system and created different classes for them.

Now I have model Order.php and OrderNew.php, Product.php and ProductNew.php and so on.

I've global settings object where property of it holds what type of class I need to instantiate.

Now I've messed a lot of code with:

if ($global->object->isNewKindOfCustomer()) { $product = new ProductNew; } else { $product = new Product; } 

But doing so in many places I got strong feeling that I do something very VERY wrong.

So my first idea was that it should be decided by class Product what kidn of product should be instantiated.

It would be perfect if I could go to the old Product class and do in its constructor and do somethign like:

Class Product { __construct() { if ($global->object->isNewKindOfCustomer()) { $product = new ProductNew; } else { $product = new Product(); } } } 

Additionally I would inherit ProductNew from Product and redefine all methods that needs to be changed. Something like:

Class ProductNew extends Product { public methodsDefinedInProductButInNeedToBeChangedForProductNew (){ } } 

But the problem is that I didn't manage to find a sane way to do it in PHP and it still smells not so good. But much better than the first approach (at least for me).

The third idea is that I will create now just ProductNew class but also ProductOld class (what doesn't exist but what I got per se).

I will move all code from current Product class to ProductOld. So my current Product class will have no methods. It will get empty. Additionally ProductNew will inherit from ProductOld.

Having such scheme I won't touch code of whole system at all. Everywhere in the code the assignemnts will have the following look as

$product = new Product 

and inside I will need somehow to manager what object will be created in Product constructor.

The fourth idea is that I will create a generic class at first and name it as e.g. ProductGeneral . It will have mehtods related to both ProductNew and ProductOld classes. They both will extend ProductGeneral. And inside Product only the proper class will be instantiated depending on the role of current user. Although I'm not sure whether ProductGeneral is so necessary....

Still no idea how I can replace this in Product class. I remember it was called something like dynamic binding in C++.

Are there any better solutions for tackling this problem? Am I missing something?

Sorry for too long text I did try to make it as short as possible.

3
  • 1
    If possible, I would create a separate namespace for those new definitions, so that you can retain the base name of Order and Product. Commented Jun 11, 2015 at 7:07
  • Also, I wouldn't make changes to the old Product class at all; instead, you could wrap the creation of those new classes into a separate factory. Commented Jun 11, 2015 at 7:09
  • could please give more details? article/a few sentences. I've opened manual about namespaces and php.net but can't get how I can bind it together Commented Jun 11, 2015 at 7:56

3 Answers 3

2
if ($global->object->isNewKindOfCustomer()) { $product = new ProductNew; } else { $product = new Product; } 

But doing so in many places I got strong feeling that I do something very VERY wrong.

I agree. That's what overriding is for.
Assimung $global->object is of type Customer, add a method newProduct that you can override as needed:

class Customer { public function newProduct() { return new Product(); } } class CustomerNew extends Customer { public function newProduct() { return new ProductNew(); } } 

Then you have to check exactly once whether you're dealing with the new customer type or the old one (when instantiating Customer or CustomerNew), and never again.
That is basically the "factory pattern", just not as a separate class.

Also, instead of having a "normal" old class and a new overriding class, I suggest you move everything the have in common to an new base class, like

abstract class CustomerBase { // Define here all methods that Customer and CustomerNew have in common // And make "implementation-dependent" methods abstract: public abstract function newProduct(); } class Customer extends CustomerBase { public function newProduct() { return new Product(); } } class CustomerNew extends CustomerBase { public function newProduct() { return new ProductNew(); } } 

This would be a "cleaner" implementation (semantically), and you still wouldn't have to replace every Customer in your code with CustomerOld or something.
I suggest you do the same thing with Product, instead of just overriding methods from the "default"

Sign up to request clarification or add additional context in comments.

Comments

1

You probably want a Product class with all the base functions, a ProductOld with the functions specific to that class, and a ProductNew with functions specific to that class. Also, a ProductFactory which will return to you the product class you need.

Without seeing the code it's hard to say whether its best to do above, or just extend the Product class and have a ProductFactory.

Comments

1

Oh no :( Its completely wrong.

Class Product { __construct() { if ($global->object->isNewKindOfCustomer()) { $product = new ProductNew; } else { $product = new Product(); } } } 

Where did you get $global? Its not in your constructor. You cant get instances from nowhere. This produces untestable code.

You are looking for the factory pattern.

3 Comments

This is an abstract code, I set a property in Yii::app()->params['kindOfUser'] whar is set upon application instantiation. (but this is not the point) Thanks for the link
I have such situation: there are two kinds of users. I can say who is who only by getting user's id and sending a request to DB for asking the meaning of one bit. I guess it's better to do such request just once when user logs in and store it globally. Do you see any better solutions?
Its not easy to say what is better (cleaner). You can after login create RegularUser or VipUser (depends on the bit) and then store the instance to session.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.