InvoiceRepositoryInterface standardization about invoicingplugin HOT 4 CLOSED

Hello Vytautas!

The point is that Sylius itself is an enormous project when compared to any of the official plugins and hence it's much harder to try something we consider fresh and interesting there (i. e. removing the get word from getters). In our opinion, that's one of the plugins' advantages. Let's set the examples of RefundPlugin and RbacPlugin - their logic relies on both application events and commands while the Sylius Core does not.

Since we've decided to use a uuid as an invoice identifier instead of an incremented integer the risk of not finding the invoice in the repository is negligible. Here comes the difference between find and get word in the repository. While find will return an invoice or null, get always returns a valid object - an invoice in this case.

from invoicingplugin.

Furthermore, set the expected return values to ?InvoiceInterface for the functions 'get' and 'getOneByOrderNumber' to avoid fatal errors if no invoice was found.

from invoicingplugin.

I totally agree with the @bartoszpietrzak1994 explanation 👍 We must remember that Sylius is developed for years and sometimes there is just no space to refactor everything according to the most modern standards. For example, interface InvoiceRepository is named this way to avoid redundant Interface suffix, when we can specify details of the class a little bit more in the implementation's name (like DoctrineInvoiceRepository). There are lot's of things that are the matter of convention and I strongly believe it's not a really big problem that they're not 100% consistent with Sylius' ones. For me, the bigger problem would be if we stick to the old, not-so-great-anymore conventions and coding principles only because we must keep them in a Sylius/Sylius repository 🚀

from invoicingplugin.

Hello @tautelis !

Please see #108 (comment) and let us know what you think and if you are fine with closing this issue as well :)

from invoicingplugin.