2

I have come up with the following design for my requirement and want know if there is a better way to do it or if you have any feedback on the design.

Requirement

Assume that an app provides different add-ons and each of these add-ons have a licence. The user is given a license key and he can activate/deactivate the license key to use that add-on.

There is also a bundle license which provides the user with a single license key that can be used to activate/deactivate all the add-ons. But there can be only one bundle license in the app.

This will be implemented in PHP.

Design

This is the design that I have come up with.

class BaseLicense {
    protected $license_key;
    protected $addon_name;

    public abstract activate();
    public abstract deactivate();
    public abstract is_active();
}

class License extends BaseLicense {
    // Inherit abstract methods
}

class BundleLicense extends BaseLicense {
    // Inherit abstract methods

    // Singleton since there can be only one bundle license.
}
Improve this question
5
  • 1
    Not sure why Class BundleLicense is having the singleton logic ? I believe in your use case, BundleLicense class instance should be tied up with the user instead of the whole app. Please clarify little more. Commented Mar 6, 2017 at 6:51
  • If the license verification code of add-on X receives an instance of BaseLicense, how does it know if that license is applicable to add-on X and not exclusively for add-on Y? Commented Mar 6, 2017 at 7:24
  • Exactly. What's missing in this question is how add-on's map to licenses. Commented Mar 6, 2017 at 7:30
  • @BartvanIngenSchenau It is verified using the addon_name property. Commented Apr 4, 2017 at 4:41
  • @meaning-matters Ha ha. No :) It was part of a WordPress plugin that I was building. Code here at github.com/sudar/email-log/tree/master/include/License Commented Apr 4, 2017 at 4:42

3 Answers 3

Reset to default
3

I think that the design is well oriented but I would sugggest the folowing:

  • Licences and add-ons are mapped via the $addon_name property but a matches(add_on_name) method should be added to the base class in order to know if a licence belongs to a given add-on, or better yet not pass the add-on name but the add-on itself so the matches method can do a more complex validation as to whether or not the license belongs to that add-on
  • Care should be taken in the BundleLicense to reduce the visibility of the inherited constructors so they cannot be used to instantiate. Since that could be tricky, a better solution would be that every parent class has their constructors private (I don't know if that can be enforced in PHP) and instead provide a getInstance() method. All non-singleton classes would return a new instance, but the singleton would return the same instance all the time. That way you don't hace to reduce the visibility of the superclasses' constructors, which I don't know if is possible in PHP.
  • A possible flaw is that BundleLicense seems to cover all add-ons, potentially giving access to add-ons it shouldn't give acces to. So I suggest classes to have a private list of add-on names intead of a single $addon_name property. Non bundle licenses would have a list containing a single add-on name, but bundle licenses will contain a list with two or more add-on names.
Improve this answer
4
  • 3
    I think it's a common misconception when the case with multiplicity one is the considered the base case and the case with multiplicity n the extension. When both are needed it is usually easier to let the 'n' case be the general case and the 1 case be the more specific one. In the implementation that follows that design usually also no special treatment needs to be implemented when switching between 1 and n. An array or list with 1 element is still a list and can be handled the same. Commented Mar 6, 2017 at 13:42
  • @HaraldScheirich Indeed. I've found many design refactorings to address the fact that usually "one" is really "a list containing only one", making it possible to have multiple items without breaking LSP. Commented Mar 6, 2017 at 13:51
  • @TulainsCórdova Excellent points. Especially the first one about a matches method. I didn't think about it and thanks for pointing it out. Commented Apr 4, 2017 at 4:47
  • Regarding your points about singleton, I found that making BundleLicense singleton was causing more problems that it was solving for me. So I ended up having a property in my main app class called $bundleLicense and then creating a instance of BundleLicense and attaching it. Commented Apr 4, 2017 at 4:49
1

A license is basically a document. It strikes me as odd that your license has methods like Activate() and Deactivate(). I would expect a LicenseManager class that does these things. The License class should do no more than encapsulate the license. It might have Read() and Write() methods to read the license from or write the license to a file but it should not contain logic that does things with licenses.

If you are really strict about it, it should not even directly read from or write to files, it should have a constructor that accepts a string and offer one or more properties that provide the license in formats for persisting it or for display. It could validate the license in the sense "this could be a valid license" and throw if it is not a valid license but it should not have knowledge of your application or environment.

The license is data. Doing the assessment whether the license is valid or not for a user, environment or timeframe and/or activating the license is a different concern that should be separated from the license itself.

Improve this answer
4
  • "A license or licence is an official permission or permit to do, use, or own something (as well as the document of that permission or permit)". The word is used for both the document and the authorization/permit, OP is using the word to mean "authorization/permit/activation key". Commented Mar 6, 2017 at 13:48
  • "The License class should do no more than encapsulate the license" yes Commented Mar 6, 2017 at 13:58
  • @Tulains Well yes. My point is that it is not a judge, not an active deciding entity or applying entity. The incarnation of the license (in whichever one of the two senses) in a computing environment will be some data. Commented Mar 6, 2017 at 14:37
  • @MartinMaat In my case, the license (key) is just a string. I agree that we should have a License class and another LicenseManager class. But since License is just a string in my case I have merged both of them into a single class. Commented Apr 4, 2017 at 4:45
1

I prefix this with that I don't know much about php and how singleton is implemented in php.

The use of singletons will always lead to discussions, some people categorically are against them, some are on the fence, some are fine. I put myself in the "on-the-fence" category. There are some places where they come in handy, for me most of the times that is when you are dealing with entities where you would like a single point of entry without having to pass something along a long chain of calls. E.g. a logging facility, memory manager... . Limiting the plurality of something from a business case (your requirements) for me is not a good use case for singletons. IMHO if you make this an explicit limitation in your code rather than using something like a singleton construct it will be much more flexible in the long run and require less change in your code when the requirements change.

Improve this answer
1
  • I agree. I found that making BundleLicense as singleton was causing more problems that it was solving. So I ended up attaching an instance to the app object and access it using that. Commented Apr 4, 2017 at 4:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.