Skip to content

Conversation

@brianjmiller
Copy link
Member

Just opening this PR to get initial feedback on things such as project setup, repo setup, core class definitions, JSON handling, anything related to Visual Studio and/or the environment. This is my first foray into Microsoft world development so would appreciate early feedback.

On the JSON handling front, we can go the serializer/deserializer route but after extensive trials with the TinCanJava lib development it ended up just being more of a PITA than it was worth and we ultimately decided on this more manual of approaches there. So, to make them match as close as possible I've continued that approach here. We can re-raise the issue if someone feels strongly that it is the wrong approach. Hopefully other than the JObject constructors and toJObject methods the rest of it is sufficiently encapsulated such to not matter.

@brianjmiller
Copy link
Member Author

Particularly interested in feedback from @bscSCORM @tseabrooks @mlefavor @oconnetf @ingramj .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we only have to support C# here, let's clean this up by using default parameters. eg:

JObject toObject(TCAPIVersion version = TCAPIVersion.latest());
toJSON(TCAPIVersion version = TCAPIVersion.latest(), bool pretty = false);

@brianjmiller
Copy link
Member Author

@bscSCORM and I took this discussion off line and I've gotten about as close to requested changes as we could determine without going down very long trail that we generally want to avoid. Any other feedback appreciated.

@brianjmiller brianjmiller self-assigned this Apr 10, 2014
* Includes LRSResponse base implementation
* Includes About and Extensions model handling
* Improvements to Version handling to provide for getting
  supported versions and getting a version given a string
* Corrections to map handling and .isEmpty method
* Add try/catch to /about call for web exception
* Add property to LRSResponse.Base for storing exception
* Add stubs for passing additional headers and query params to GetRequest
@emmacadence
Copy link

This is more a question for my own education than anything else: Why the preference for using JObjects, rather than just letting Json.NET do it's regular magic serialization/deserialization based on fields and properties? Is it a performance concern? Design concern?

@brianjmiller
Copy link
Member Author

See original comment in PR. Mostly, after going back and forth when building the TinCanJava lib it was more pain than it was worth because of the need to support multi-version.

@emmacadence
Copy link

D'oh. I can't believe I missed that comment. I will say that I found Json.NET's handling of subclass dispatch (which I assume was the trouble you had with multi-version) to be much easier to configure than Jackson's (which I'm assuming you used for Java). But keeping the design close to the way Java was designed is probably more important.

* Adds overall structure to handle synchronous HTTP requests
@brianjmiller
Copy link
Member Author

Closing because I'm rewriting history to correct author info. Will re-open for actual merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants