search

bunq / sdk_php

PHP SDK for bunq API
MIT License
84 stars 54 forks source link

Implement interfaces/inheritance #151

Open Wouter0100 opened 6 years ago

Wouter0100 commented 6 years ago

This isn't a real issue - rather a suggestion or discussion. Would it be an idea to more implement inheritance or general interfaces?

For example currently the getReferencedObject in User returns a Model (according to the php docblock), which does not the describe the methods that all the User models has in common. This would ease up implementation a lot.

The scenario is for example that the getDisplayName is implemented by all models (UserCompany, UserLight, UserPerson) but no general interface which describes the implementation.

OGKevin commented 6 years ago

This is indeed a good suggestion. I will see if its possible to generate these as well. However, this will be hard for the anchor objects. As these will have almost nothing in common. I can only think of this working for the user endpoint tbh 🤔 As en example see https://github.com/bunq/sdk_php/blob/98e52ccedbe2a01ca2c047e1d5d893016dd14873/src/Model/Generated/Object/NotificationAnchorObject.php#L572 No interface will make this easier to type hint 🤔

Wouter0100 commented 6 years ago

Oh - darn. No, I don't think that's useful indeed. Maybe I'll see a way when working some more with the SDK, but currently the user would be useful.

Wouter0100 commented 6 years ago

I see the same for the Monetary accounts. I'm currently making a connection with bunq where it does not matter if it is an Light, Company or Personal monetary or user. So general things like getId() in both monetary and user would be useful to implement.

OGKevin commented 6 years ago

@Wouter0100 why could the user not use a simple if (instaceof xxx) statement ? IMO that would work better as you expect which instance your code expects and can have better error handling. In your case, you would need to user || statement which might not be nice 🤔

Wouter0100 commented 6 years ago

My code expects a user - it does not really matter which user. Introducing if-statements indicates a code smell and is against a wide variety of design patterns/modularity principles.

OGKevin commented 6 years ago

fair enough. If you can create an MR that does the following than I'm happy to merge it.

  1. Create an interface with the common properties.
  2. Create a little script that basically modifies the generated code to append the calls signature e.g. User implements SomeInterface and adds the correct use statements

The reason i dont want to add this to the generator is because it will increase its complexity as i explained above, not all getReferencedObject would work, and therefore introduces extra logic to determine when to generate and use an interface and when not.