WorkGroup Data Service with JSON / Web based API

I'm wondering if this code can be made more clear and fluent:

```
using System;
using System.Threading.Tasks;
using Windows.Data.Json;
using System.Collections.ObjectModel;
using System.Net.Http;
using System.Collections.Generic;
using Newtonsoft.Json;
using System.Linq;
using eTest.Space.Log;
using eTest.Space.Interface;
using eTest.Objects;

namespace eTest.Space.Services
{

public class WorkGroupDataService : IWorkgroupDataService
{

public string WebApiUri
{
get { return Constants.ServerAddress; }
}

private ObservableCollection Groups;// = new ObservableCollection();

public async Task> GetAllWorkGroups()
{
Groups = new ObservableCollection();
//if (Groups.Count == 0)
{
Uri dataUri = new Uri(WebApiUri + "api/workgroup/");

using (var client = new Windows.Web.Http.HttpClient())
{
var response = await client.GetAsync(dataUri).AsTask(); ;

string jsonText = response.Content.ReadAsStringAsync().GetResults();
JsonArray jsonArray = JsonArray.Parse(jsonText);

foreach (JsonValue groupValue in jsonArray)
{
JsonObject groupObject = groupValue.GetObject();
IWorkgroup group = new Workgroup();
group.GroupState = (int)groupObject["GroupState"].GetNumber();
group.WorkgroupName = groupObject["WorkgroupName"].GetString();
group.WorkgroupSecretary = groupObject["WorkgroupProba"].GetString();
group.ObjectID = groupObject["ObjectID"].GetString();
Uri dataUri1 = new Uri(WebApiUri + "api/etest/" + group.ObjectID);
using (var client1 = new Windows.Web.Http.HttpClient())
{
var response1 = await c

Naming Conventions

Private instance fields should be lowercase:

private ObservableCollection groups;

Local variables should also be lowercase:

List test = JsonConvert.DeserializeObject>(jsonText1);

Also, where you are doing the same thing with the clients and URIs, you are adding numbers to the second of those variables, e.g. client1. Try to think of something more descriptive, those two clients are serving two different purposes so try something like purposeClient where purpose is replaced by a word for it's purpose. At this point, extracting the functionality out into separate methods is probably a good idea.

Improvements

If you're going to use an async method, then you should await it:

var jsonText = await response.Content.ReadAsStringAsync().GetResults();

Structure

Similarly, given that this variable doesn't look like it needs to be shared amongst methods (unless you haven't included the rest of your class), I'm not really sure why you've made it an instance field and can quite comfortably be declared inside your GetAllWorkGroups() method.

You seem to assign a value to Groups twice in that method, you don't need to do the second one:

groups = groups.OrderByDescending(x => x.Test.Count);

You also seems to double wrap the OrderByDescending clause in brackets which was unnecessary

You seem to be switching between using var and, for example string. A lot of people do switch, using var where the type inferred is obvious and using the type when it is not immediately obvious to make it more readable (this is what I do), but a lot of people will exclusively use var. In terms of style, I'd definitely go one way or the other because currently your switching just seems messy.

I find that object initializers make the code more readable so here:

IWorkgroup group = new Workgroup();
group.GroupState = (int)groupObject["GroupState"].GetNumber();
group.WorkgroupName = groupObject["WorkgroupName"].GetString();
group.WorkgroupSecretary = groupObject["WorkgroupProba"].GetString();
group.ObjectID = groupObject["ObjectID"].GetString();

Can be:

IWorkgroup group = new Workgroup 
{
    GroupState = (int)groupObject["GroupState"].GetNumber(),
    WorkgroupName = groupObject["WorkgroupName"].GetString(),
    WorkgroupSecretary = groupObject["WorkgroupProba"].GetString(),
    ObjectID = groupObject["ObjectID"].GetString()
};

After this point, you then start a new using, but from the code it seems that you are in fact finished using your previous client so you can move it around to be more like this:

public async Task> GetAllWorkGroups()
{
    var groups = new ObservableCollection();
    JsonArray jsonArray;

    using (var client = new Windows.Web.Http.HttpClient())
    {
        var dataUri = new Uri(WebApiUri + "api/workgroup/");
        var response = await client.GetAsync(dataUri).AsTask();

        var jsonText = await response.Content.ReadAsStringAsync().GetResults();
        jsonArray = JsonArray.Parse(jsonText);
    }

    foreach (var groupValue in jsonArray)
    {
        var groupObject = groupValue.GetObject();
        IWorkgroup group = new Workgroup
        {
            GroupState = (int)groupObject["GroupState"].GetNumber(),
            WorkgroupName = groupObject["WorkgroupName"].GetString(),
            WorkgroupSecretary = groupObject["WorkgroupProba"].GetString(),
            ObjectID = groupObject["ObjectID"].GetString()
        };

        var dataUri = new Uri(WebApiUri + "api/etest/" + group.ObjectID);
        using (var client = new Windows.Web.Http.HttpClient())
        {
            var response = await client.GetAsync(dataUri).AsTask();
            if (response.StatusCode == Windows.Web.Http.HttpStatusCode.Ok)
            {
                var jsonText = await response.Content.ReadAsStringAsync().GetResults();
                var test = JsonConvert.DeserializeObject>(jsonText);
                group.Test = test;
            }
        }

        if (group.Test.Count > 0)
        {
            groups.Add(group);
        }
    }

    return groups.OrderByDescending(x => x.Test.Count);
}

StackExchange Code Review Q#71500, answer score: 7