Feedback

[motin]

Hey guys, nice to see community work being done on this library :)

Here is some feedback and questions. For the ones that we find needs more discussion, we'll create separate issues.

1. Please provide what the goal of this fork is.

For instance, this seems to be a continuation of billogram#16 and an effort to bring the lib up to par with modern php standards. It seem to cater to more experienced php developers. Is there some other purpose of this fork that is good to state in the readme?

2. Please provide examples of how to use the new features of this version vs the upstream version, for instance to log, profile or debug the HTTP requests sent using this library. Linking to upstream docs in the README would do fine.

3. Is there any particular reason to use final and private keywords in this lib? For application-specific code it'd be justified, but since this is a library, you'll expect users to want to extend (in the general sense) your code in ways which you would never foresee without necessarily having to fork the lib.

One example would be if I want to extend the Api\Item class, then I'd also like to extend the BillogramClient class to return my extended class in the items() method. This is not possible with final+private keywords found in that class.

4. The typo "costumer" instead of "customer" appears here and there in the code.

5. The README is out of date in both how to install the package, how to create the api client and set auth credentials (api key or api password for instance?). Let us now when this is updated so that it is easier to try the new lib :) Thanks

[Nyholm]

Thank you for this issue.
We have been developing this library slowly and "under the radar". The plan is to be done within a month. By "done" I mean, properly done with docs and all.

I will address all your comments in the readme and code. But here are some quick answers:

1. Yes, I/we was not happy with the maintainer of the original package. We want to create a modern API client. If the original maintainers wants to merge this fork, nobody would be happier than us.

2. Naturally. Thanks!

3. No, not really. I think is is good practise. If anybody comes up with a good reason to remove the final then Im happy to do it. I do think one should use composition over inheritance. We do also strive to be SOLID enough so you do not need to extend our final classes. But as I said. I do not really care if it is final or not.

4. Thanks!

5. I will. Thank you!

[motin]

Thanks for the quick reply, makes sense. Regarding the final and private keywords, my vote is to remove them until having them gives a clear benefit. Sometimes inheritance is unavoidable despite the best intentions, and I have bad memories of having to copy-paste/fork code just to work around a small unintended bug :) I'll close this issue as it is not meant for tracking the individual concerns, just for giving feedback.