search

Setono / sylius-tier-pricing-plugin

Add tier pricing to your Sylius store
MIT License
0 stars 1 forks source link

Rename interface and trait #4

Open leflings opened 3 months ago

leflings commented 3 months ago

Describe the proposed solution

I suggest we rename the following

(alternatively something like ProductWithPriceTiersInterface/Trait)

It just seemed a little weird to just have use ProductTrait and implements ProductInterface. Kind of nondescript.

If we can agree on a naming, I'll make the PR if you wish :)

Describe alternatives you've considered

No response

Additional context

No response

loevgaard commented 3 months ago

I think it's a good idea. I have always done this because of conventions. Actually I first named them PriceTiersAwareInterace, but then it seemed wrong to extend the ProductInterface, but with your approach we could still extend those. Maybe PriceTiersAwareProductInterface. The reason I like the '...Aware' is because it's widely used with the Sylius and Symfony code bases.

As a side note, the upside of having just ProductInterface is that it's generic and you can have things not directly related to price tiers on that interface, but for now we don't have that, so I am okay with renaming.

leflings commented 3 months ago

I mean, we can also just leave it as is, and let people make their own aliases when using them in their own Product file.

That's what you'd end up doing anyway, if you had several plugins exposing a ProductInterface

I like the ...Aware aswell. I think it works best though, if the interface/trait can be used on several different entities, which this one can't really, since the PriceTier model is closely tied to Product.

Anyway - it's your call... Maybe I just missed writing github issues ;)