基于JSON / Web的WorkGroup数据服务

Question

我想知道这段代码是否可以变得更清晰、更流畅：

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<IWorkgroup> Groups;// = new ObservableCollection<IWorkgroup>();

        public async Task<ObservableCollection<IWorkgroup>> GetAllWorkGroups()
        {
            Groups = new ObservableCollection<IWorkgroup>();
            //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 client.GetAsync(dataUri1).AsTask();
                            if (response1.StatusCode == Windows.Web.Http.HttpStatusCode.Ok)
                            {
                                string jsonText1 = response1.Content.ReadAsStringAsync().GetResults();
                                List<TestSession> Test = JsonConvert.DeserializeObject<List<TestSession>>(jsonText1);
                                group.Test = Test;
                            }
                        }
                        if (group.Test.Count > 0)
                        {
                            Groups.Add(group);
                        }

                    }
                }
            }
            Groups = new ObservableCollection<IWorkgroup>(Groups.OrderByDescending((x => x.Test.Count)));
            return Groups;
        }
    }
}

Answer 1

命名约定

私有实例字段应该是小写：

private ObservableCollection<IWorkgroup> groups;

局部变量也应是小写：

List<TestSession> test = JsonConvert.DeserializeObject<List<TestSession>>(jsonText1);

另外，在对客户端和URI执行相同的操作时，您要将数字添加到第二个变量中，例如client1。试着想一些更具描述性的东西，这两个客户端服务于两个不同的目的，所以尝试类似于purposeClient这样的东西，其中的目的被一个词替换为它的目的。此时，将功能提取到单独的方法中可能是个好主意。

改进

如果要使用async方法，则应该使用await方法：

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

结构

类似地，考虑到这个变量看起来不需要在方法之间共享(除非您还没有包含类的其他部分)，我不太清楚为什么要将它变成实例字段，并且可以在GetAllWorkGroups()方法中轻松地声明它。

在该方法中，您似乎为Groups分配了两次值，不需要执行第二个方法：

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

您似乎还将OrderByDescending子句双包装在括号中，这是不必要的。

您似乎在使用var和string之间进行切换。很多人确实会切换，在推断类型明显的情况下使用var，当不是立即使其更加可读性时使用该类型(这就是我所做的)，但很多人将只使用var。就风格而言，我肯定会采取这样或那样的方式，因为目前你的切换看起来很混乱。

我发现对象初始化器使代码更具可读性，因此在这里：

IWorkgroup组=新工作组()；group.GroupState = (int)groupObject“GroupState”.GetNumber()；group.WorkgroupName = groupObject“WorkgroupName”.GetString()；group.WorkgroupSecretary = groupObject“WorkgroupProba”.GetString()；group.ObjectID = groupObject“ObjectID”.GetString()；

可以是：

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

在此之后，您将启动一个新的using，但是从代码中可以看出，您实际上已经使用了以前的客户端，因此您可以将它移动到如下所示：

public async Task<ObservableCollection<IWorkgroup>> GetAllWorkGroups()
{
    var groups = new ObservableCollection<IWorkgroup>();
    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<List<TestSession>>(jsonText);
                group.Test = test;
            }
        }

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

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

Answer 2

基于丹鲁尔's的好回答

如果是group.Test.Count == 0，将跳过IWorkgroup group完成的所有工作。所以你最好改变执行流程。

foreach (var groupValue in jsonArray)
{
    var groupObject = groupValue.GetObject();
    String objectId = groupObject["ObjectID"].GetString();

    var test = GetTestSessions(objectId);
    if (test.Count == 0) { continue; }

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

    groups.Add(group);
}

调用提取的重载方法

private List<TestSession> GetTestSessions(String objectId)
{
    return GetTestSessions(new Uri(WebApiUri + "api/workgroup/" + objectId));
}

private List<TestSession> GetTestSessions(Uri dataUri)
{
    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();
            return JsonConvert.DeserializeObject<List<TestSession>>(jsonText);
        }

        return new List<TestSession>();
    }

}

Answer 3

绝对一点儿没错!在C#中有很多很酷的捷径。我将把它们应用到代码中，并对它们进行注释：

namespace eTest.Space.Services
{
    public class WorkGroupDataService : IWorkgroupDataService
    {
        public string WebApiUri
        {
            get { return Constants.ServerAddress; }
        }

        public async Task<ObservableCollection<IWorkgroup>> GetAllWorkGroups()
        {
            // WARNING: You have a private member that has the potential to be 
            // shared across threads if GetAllWorkGroups is used asynchronously!
            // Swapped for a local variable instead.
            var groups = new ObservableCollection<IWorkgroup>();
            //if (groups.Count == 0)
            {
                // Love the var keyword, means typing the Type once instead of twice
                var dataUri = new Uri(WebApiUri + "api/workgroup/");

                string jsonText;
                using (var client = new Windows.Web.Http.HttpClient())
                {
                    // AsTask redundant, already a task
                    // ConfigureAwait to avoid unnecessary context switch (you'll await again outside this and can choose to keep context or not depending on if you do work in the UI
                    var response = await client.GetAsync(dataUri).ConfigureAwait(false);

                    // we're using async, await when getting results to trigger thread release while waiting
                    jsonText = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
                // We close the using as early as possible to release the object as early as possible
// Also, the less nesting of scopes, the easier your code is to read
                }

                foreach (var groupValue in jsonArray)
                {
                    var groupObject = groupValue.GetObject();

                    // Moved body of foreach to its own method
                    var group = GetGroupFromGroupObject(groupObject);

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

            // Removed the private member assign, returned the local variable
            return groups;
        }

        private Group GetGroupFromGroupObject(GroupObject groupObject)
        {
            // Object initializer for cleaner syntax
            var group = new Workgroup
            {
                GroupState = (int)groupObject["GroupState"].GetNumber();
                WorkgroupName = groupObject["WorkgroupName"].GetString();
                WorkgroupSecretary = groupObject["WorkgroupProba"].GetString();
                ObjectID = groupObject["ObjectID"].GetString();
            }

            // string.format reads better
            // dropping args to their own lines can improve readability
            var uriString = string.Format(
                                "{0}api/etest/{1}", 
                                WebApiUri, 
                                group.ObjectID);

            // Avoiding nested calls can make for easier reading
            var dataUri1 = new Uri(uriString);

            using (var client1 = new Windows.Web.Http.HttpClient())
            {
                var response1 = await client.GetAsync(dataUri1).ConfigureAwait(false);
                if (response1.StatusCode == Windows.Web.Http.HttpStatusCode.Ok)
                {
                    var jsonText1 = await response1.Content.ReadAsStringAsync().ConfigureAwait(false);

                    // No need to create a separate var just to assign
                    // Type T will be inferred from the object, no need to write it
                    group.Test = JsonConvert.DeserializeObject(jsonText1);
                }
            }

            return group;
        }
    }
}