Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement interfaces/inheritance #151

Open
Wouter0100 opened this issue Jun 7, 2018 · 6 comments
Open

Implement interfaces/inheritance #151

Wouter0100 opened this issue Jun 7, 2018 · 6 comments

Comments

@Wouter0100
Copy link

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 OGKevin modified the milestones: 0.13.5, 1.0.0, 1.1.0 Jun 7, 2018
@OGKevin
Copy link
Contributor

OGKevin commented Jun 7, 2018

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

public function getReferencedObject()
No interface will make this easier to type hint 🤔

@Wouter0100
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

OGKevin commented Jun 17, 2018

@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
Copy link
Author

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
Copy link
Contributor

OGKevin commented Jun 18, 2018

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.

@angelomelonas angelomelonas removed this from the 1.1.0 milestone Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants