dknoodle / WUnderground.Net

WUnderground .Net Library (Also Weather.com)
MIT License
19 stars 13 forks source link

GetHourly API does not work properly #4

Closed Gosha24 closed 7 years ago

Gosha24 commented 7 years ago

I Uncommented GetHourly API call in Sample and get Temperature Dewpoint etc zeros. Then I simply erased new DoubleConverter in JsonConvert.DeserializeObject(response.Content, new BoolConverter()); call and it fixed the issue. Something is wrong with DoubleConverter. I want to mention that I like this library, and will be happy to see this bug fixed. I also hope that author will provide: 1) GetHourlyAsync and GetHourly10DayAsync API, which is simple. In my code i did it with copy/paste 2) xml interface in parallel to json interface

dknoodle commented 7 years ago

Thanks for the feedback!

I have updated Json.Net to the newest version which removed the need for the DoubleConverter, this should be fixed now. The project has been moved to .NetStandard to support additional framework versions as well.

GetHourlyAsync was already in the project so I'm not sure what you need there. I added GetHourly10Day so that should be there now.

Is there a reason for having XML support? Using XML would dramatically increase the data going over the network for any client and I really don't see a purpose. I see several parts where this would add more overhead with no added benefit. Can you elaborate please?

Thanks, Daniel

Gosha24 commented 7 years ago

Hi Daniel Thank you for your answer and also for development and support of your project. 1) I do not think that XML can add more than 1 -2 K overhead. In modern internet it is negligible value. In my experiments for files 1k or 10k is just the same. Bottleneck is in open internet connection. 2) It is better to have XML option because XML processing is standard in C# programming and is broadly used by majority of developers. For users of your project it would save development time, which in my opinion is more important than execution time of application. I asked for XML support exactly due to this consideration. Yours Ilya Shpilberg

On Mon, Apr 17, 2017 at 6:56 PM, Daniel Knoodle notifications@github.com wrote:

Closed #4 https://github.com/dknoodle/WUnderground.Net/issues/4.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dknoodle/WUnderground.Net/issues/4#event-1045145679, or mute the thread https://github.com/notifications/unsubscribe-auth/AQBLkiWYduiQ1FACzgCIQnbr0iTY2IKbks5rw4uwgaJpZM4MUBLj .

--

  *Ilya Shpilberg*

      *Home*:               077-3088133

  *Mobile:*              0544-267545
dknoodle commented 7 years ago

Hi Ilya,

I appreciate the feedback but I have to disagree with the overhead being negligible. Adding XML tags around every element in the response body can multiply the data going over the wire by several factors of size. In mobile applications this can make a big performance difference especially if the users' cellular connection is weak. Also most developers these days actually use JSON format rather than XML. Even visual studio projects themselves are moving from XML based configuration files to JSON based configuration files for a number of reasons.

Regardless of the data size/usage debate though, I believe it's irrelevant for the purposes of this library. Retrieving data from Weather Underground in JSON or XML format makes no difference as the end result to the consuming application is a Class object. The entire point of having a wrapper project is to abstract away all of that from the developer so your code can work directly with strongly typed objects. Supporting or not supporting XML does nothing for development time (faster or slower) as the library wraps the API call and gives you a strongly typed object for use in your code.

Is there some scenario where you would need to work with raw XML or even know the library was using XML inside its code that I'm missing? I'm still struggling to see any purpose for supporting XML.

Thanks, Daniel