softwerkab / fortnox-csharp-api-sdk

.NET SDK for Fortnox API.
MIT License
52 stars 64 forks source link

Create a constructor for the connectors #156

Closed vanillajonathan closed 3 years ago

vanillajonathan commented 3 years ago

Create a constructor for CustomerConnector, VoucherConnector, etc that takes in a client secret and a access token.

This makes it easier to use since the developer will know what is expected to be passed in.

richardrandak commented 3 years ago

Constructors were added for FortnoxClient. However, default (empty) constructor is kept to support the "object initializer" usage

All the connectors were internalized and only interfaces are accessible, so the constructors for connectors are no longer needed.

vanillajonathan commented 3 years ago

This is good!

Can we get more constructors for other classes too? That would help guide developers in how to use the classes. Using a constructor is much easier than using object initializer or setting properties, and it is safer since you know exactly what needs to be set.

Perhaps the Voucher class could have two static factory methods; one to create a credit and another one to create debit.

richardrandak commented 3 years ago

I don't think I have enough domain knowledge to decide what needs to be set for the voucher or other entities :) It is too complex and many different use cases, so I keep it open for users to have full control of entity initialization.

However, if you have suggestion, maybe some common use cases, please suggest. I am not sure what you mean by credit and debit voucher, I mean, voucher is always balanced, so it contains both credit and debit rows. We could do some kind of extensions with factories for common cases.

vanillajonathan commented 3 years ago

Yeah, that's the problem, domain knowledge. As a developer it can be difficult to use the API if you lack domain knowledge because then you don't know what properties to set, so constructors or factory methods could help by making the API easier to use.

https://developer.fortnox.se/documentation/resources/vouchers/ Take the voucher example, the documentation states that the Description and TransactionDate fields are required but since the Voucher class have no constructor it becomes possible to initialize an instance that is an invalid state. It makes it difficult for developers to use the class when the developer don't know what is expected or required.