karlwancl / GoogleCloudPrintApi

A .NET wrapper for Google Cloud Print API, based on .NET standard 1.4
MIT License
12 stars 14 forks source link

Added ability to search printers and submit a print job #12

Closed elacy closed 7 years ago

elacy commented 7 years ago

This is the shape of what I want to add, I need to do some more testing before it's fully ready to go but I'd be interested in your opinion on it before I get to that point, particularly around contract serialization. Ideally I'd like to see all those constructors gotten rid of so you don't have to pass in a bunch of nulls for everything, it would result in cleaner code overall afaik. Also we could probably use inheritance to make it so that all the Response classes don't have duplicated code etc.

karlwancl commented 7 years ago

For the contract deserialization, you're right. I must admit that the initial design is a little bit verbose and not clever enough, I agree that we should make some modifications to remove the needs of including a constructor in every object for deserialization.

The initially design of the constructor is to provide an entrance for JSON.NET to assign values of keys to the readonly properties with matched property names.

For matching, I see your code using ContractResolver which I think it's a solution to this part.

For assignation of readonly property, I find there's also a global setting from this which I think it's a good way to start with.


For the response classes inheritance, you're also right. I didn't notice there is a common set of properties when I create those classes, maybe I was day-dreaming when doing these kind of copy & paste works, haha.

To take it further, we may use a generic base class for the response classes, let's say ResponseBase<T> where T: IRequest, as I discovered that the common "Request" property in the Response classes should reflect the original Request classes which we may be able to use a static type instead of dynamic type to store the request.

karlwancl commented 7 years ago

I have just committed a new version of the api, it should solve the readonly property assignation issue when deserializing the response to response classes. It's not tested yet but it should be the right direction to go. Please let me know if you have any question or suggestion on the modification. Thanks.