[Product Variant] Price Calculator fixed to return always int

[lruozzi9]

Q	A
Branch?	1.8
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Related tickets	no
License	MIT

This Pull Request will avoid returning null on int only return type allowed function. If getPrice is null for the channel the same exception is thrown.

[vvasiloi]

Hey @lruozzi9, thank you for your contribution!
Could you add some tests to cover these changes?

[lruozzi9]

Sure 👍🏻

[vvasiloi]

@lruozzi9 Great! Thank you!
WDYT about changing the conditions to null !== X and returning early, then throw an exception at the end.
This will remove some unnecessary nesting...
Also, the logic to construct the error message is duplicated.
Maybe extract it to a factory method on the exception class, something like createForVariant?

[lruozzi9]

I agree, it is more readable! I will change it.

[vvasiloi]

I would like you to make a few more changes if you don't mind.
I promise it's the last one from me.

[lruozzi9]

Hey @vvasiloi! Sorry, I was a little busy these days! I will check your comments!

[lruozzi9]

I have a doubt about the location of the factory exception folder. Perhaps it is more correct to create a Factory folder inside Exception? Or an Exception folder inside Factory?

[vvasiloi]

There's really no right or wrong here, it's all a matter of opinion, especially because there's no precedent for it in Sylius' codebase.
Let's have the factory in the same namespace as the exception class, that's what I saw others practice and it seems convenient.

[vvasiloi]

On the other hand, we only have one case and I don't it changing anytime soon, so maybe it's better to just put the factory method inside the exception class.

[vvasiloi]

@lruozzi9 I just noticed that you target branch 1.8 🤦‍♂️. Can you rebase your branch and change the target to 1.10?

[GSadee]

Thanks, Lorenzo! 🎉