Shouldn't `getFeatureByTag()` throw an Exception?

[boryn]

Now, if someone queries a non-existent feature, e.g. $subscription->getFeatureByTag('test'), they get null in response. But maybe getFeatureByTag() should return an exception as an analogy to PlanSubscriptionNotFound?

[bpuig]

Not really, package is consistent with getXByY being a mimic of first(). You suggest changing to mimic firstOrFail. Any examples of a packages using exceptions for this cases or doc references? I'm a bit on the dark here.

[boryn]

Hmmm... I don't have any other examples really. More I was looking at the behaviour of what already there is in the library. Calling $user->subscription() we have in the HasSubscriptions.php trait:

$subscription = $this->subscriptions()->where('tag', $subscriptionTag)->first();

if (!$subscription) {
    throw new PlanSubscriptionNotFound($subscriptionTag);
}

so that's why I suggested throwing an exception as well with $subscription->getFeatureByTag('test').

Additionaly when I run $subscription->canUseFeature('blabla'); I get false but it's unclear whether the blabla feature exists at all or not. With the exception it would be clear there is no such a feature. Similarly, with $subscription->getFeatureUsage('blabla') or $subscription->getFeatureRemainings('blabla');.

[rodrigolopespt]

Hey guys. So nice to find this repo. Will use it and try to contribute too. In this particulary case i would go with custom exceptions but can agree with you both, just be consistent with the path you pick so we dont have to deal with different behaviours.

Thank you both!

[bpuig]

I've been looking in other packages what Exceptions do they return. I think it's ok to return null and mimic first(). We'll follow this route from now on and also keep exceptions to the bare minimum.