5
\$\begingroup\$

I have this class with two public async methods, which also calls some private methods that are async.

Here's the class:

public class AggregatedDataService
{
    public IPrincipal User
    {
        get { return HttpContext.Current.User; }
    }

    public object SessionToken { get { return HttpContext.Current.Session["session_token"]; } }
    private static string baseUrl = ConfigurationManager.AppSettings["baseURI"];
    private readonly ILog _log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType);

    public async Task<OrganizationAggregatedInfo> GetOrganizationAggregatedInfo(int organizationId)
    {
        var organization = await GetOrganization(organizationId);
        var organizationContact = await GetOrganizationContact(organization.ID);
        var associations = await GetOrganizationAssociations(organization.ID);

        return new OrganizationAggregatedInfo(organization.Name, organization.Address, organizationContact,
            associations);
    }

    public async Task<SchoolAggregatedInfo> GetSchoolAggreagtedInfo(int schoolId)
    {
        if(schoolId < 1)
            return null;

        var school = await GetSchool(schoolId);
        if (school == null)
        {
            return null;
        }
        var getSchoolAddressTask = GetSchoolAddress(schoolId); 
        var getMemberSchoolsTask = GetMemberSchools(schoolId);
        var getSchoolCurriculumTask = GetSchoolCurriculum(schoolId);
        var getSchoolFacilitiesTask = GetSchoolFacilities(schoolId);
        var getSchoolAssociationsTask = GetSchoolAssociations(schoolId);
        var getSchoolAcreditationsTask = GetSchoolAcreditations(schoolId);
        var getGovernanceStructureTask = GetGovernanceStructure(schoolId);

        await Task.WhenAll(getSchoolAddressTask, getMemberSchoolsTask, getSchoolCurriculumTask,
                getSchoolFacilitiesTask, getSchoolAssociationsTask, getSchoolAcreditationsTask,
                getGovernanceStructureTask);

        var schoolAddress = getSchoolAddressTask.Result;
        var memberSchools = getMemberSchoolsTask.Result; 
        //var teacherInfo = await GetTeacherInformation(schoolAddress.SchoolID);
        var curriculum = getSchoolCurriculumTask.Result;
        var facilities = getSchoolFacilitiesTask.Result;
        var associations = getSchoolAssociationsTask.Result;
        var accreditations = getSchoolAcreditationsTask.Result;
        var governanceStructure = getGovernanceStructureTask.Result; 

        SchoolContactReadView contact = null;

        if (schoolAddress != null)
        {
            if (schoolAddress.SchoolContact != null)
                contact = SchoolContactReadView.ShowSchoolContactView(schoolAddress.SchoolContact);
        }

        var schoolAggregateInfo = new SchoolAggregatedInfo
        {
            Name = school.Name,
            Address = school.Address,
            MemberSchoolsCount = memberSchools.Count(),
            GovernanceStructure = governanceStructure,
            Accreditations = accreditations.ToList(),
            Associations = associations.ToList(),
            Contact = contact,
            Curriculum = curriculum,
            Facilities = facilities.ToList()
        };

        return schoolAggregateInfo;
    }

    private async Task<List<OrganizationAssociationReadView>> GetOrganizationAssociations(int organizationId)
    {
        List<OrganizationAssociationReadView> associations = new List<OrganizationAssociationReadView>();

        using (var client = new HttpClient())
        {
            if (SessionToken != null)
            {
                try
                {
                    if (ValidateToken(client).IsSuccessStatusCode)
                    {
                        HttpResponseMessage response = await client.GetAsync("api/organization/associations/" + organizationId);
                        if (response.StatusCode == HttpStatusCode.OK)
                        {
                            //create list of associations
                            var contentResult = await response.Content.ReadAsAsync<List<OrganizationAssociation>>();
                            associations = contentResult.Select(OrganizationAssociationReadView.MapFrom).ToList();
                        }
                        else
                        {
                            _log.Error("API Error Reason: " + response.ReasonPhrase);
                        }
                    }
                }
                catch (Exception sd)
                {
                    _log.Error("Error displaying Associations ", sd);
                }
            }
            else
            {
                _log.Info("User not logged-in.");
            }
        }

        return associations;
    }

    private async Task<OrganizationContactReadView> GetOrganizationContact(int organizationId)
    {
        OrganizationContactReadView organizationContact = null;

        using (var client = new HttpClient())
        {
            if (SessionToken != null)
            {
                try
                {
                    if (ValidateToken(client).IsSuccessStatusCode)
                    {
                        HttpResponseMessage response = await client.GetAsync("api/organization/contact/" + organizationId);
                        if (response.StatusCode == HttpStatusCode.OK)
                        {
                            var contentResult = await response.Content.ReadAsAsync<OrganizationContact>();
                            var organizationContactReadView = OrganizationContactReadView.MapFrom(contentResult);

                            organizationContact = organizationContactReadView;
                        }
                        else
                        {
                            _log.Error("API Error Reason: " + response.ReasonPhrase);
                        }
                    }
                }
                catch (Exception sd)
                {
                    _log.Error("Error displaying Organization Contact Information ", sd);
                }
            }
            else
            {
                _log.Info("User not logged-in.");
            }
        }

        return organizationContact;
    }

    private async Task<OrganizationReadView> GetOrganization(int organizationId)
    {
        OrganizationReadView organization = null;

        using (var client = new HttpClient())
        {
            if (SessionToken != null)
            {
                try
                {
                    if (ValidateToken(client).IsSuccessStatusCode)
                    {
                        HttpResponseMessage response = await client.GetAsync("api/organization/getbyid/" + organizationId);
                        if (response.StatusCode == HttpStatusCode.OK)
                        {
                            var responseContent = await response.Content.ReadAsAsync<Organization>();
                            var organisationReadView = OrganizationReadView.MapFrom(responseContent);
                            organization = organisationReadView;
                        }
                        else
                        {
                            //make sure the server has this installed and supports c# 6
                            //_log.Info($"Failed to get organization because the hhtp response was not OK. Response code is {response.StatusCode.ToString()}");
                            _log.Info("Failed to get organization because the hhtp response was not OK. Response code is " + response.StatusCode);
                        }
                    }
                }
                catch (Exception ex)
                {
                    //make sure the server has this installed and supports c# 6
                    //_log.Error($"An error occured while trying to retrieve Organization for user {User.Identity.Name}. Exception message: {ex.Message}");
                    _log.Error("An error occured while trying to retrieve Organization for user" +
                               User.Identity.Name + ". Exception message: " + ex.Message);
                }
            }
            else
            {
                _log.Info("User not logged-in.");
            }

            return organization;
        }
    }

    private async Task<GovernanceStructureReadView> GetGovernanceStructure(int schoolId)
    {
        GovernanceStructureReadView governanceStructureReadView = new GovernanceStructureReadView();
        using (var client = new HttpClient())
        {
            if (SessionToken != null)
            {
                try
                {
                    if (ValidateToken(client).IsSuccessStatusCode)
                    {
                        var response = await client.GetAsync("api/school/getgovernancestructure/" + schoolId);
                        if (response.StatusCode == HttpStatusCode.OK)
                        {
                            var responseContent = await response.Content.ReadAsAsync<GovernanceStructure>();
                            governanceStructureReadView =
                                GovernanceStructureReadView.ShowGovernanceStructureView(responseContent);
                        }
                        else
                        {
                            _log.Error("API Error Reason: " + response.ReasonPhrase);
                        }
                    }
                }
                catch (Exception sd)
                {
                    _log.Error("Error displaying Governance Structure ", sd);
                }
            }
            else
            {
                _log.Info("User not logged-in.");
            }
        }
        return governanceStructureReadView;
    }

    private async Task<IEnumerable<AccreditationReadView>>  GetSchoolAcreditations(int schoolId)
    {
        IEnumerable<AccreditationReadView> accreditationReadViews = new List<AccreditationReadView>();
        using (var client = new HttpClient())
        {
            if (SessionToken != null)
            {
                try
                {
                    if (ValidateToken(client).IsSuccessStatusCode)
                    {
                        HttpResponseMessage response =
                            await client.GetAsync("api/school/accreditationsbyschool/" + schoolId);
                        if (response.StatusCode == HttpStatusCode.OK)
                        {
                            var accreditations = await response.Content.ReadAsAsync<List<Accreditation>>();
                            accreditationReadViews =
                                accreditations.Select(AccreditationReadView.ShowAccreditationView);
                        }
                        else
                        {
                            _log.Error("API Error Veiwing Accreditations : " + response.ReasonPhrase);
                        }
                    }
                }
                catch (Exception sd)
                {
                    _log.Error("Error displaying Accreditations ", sd);
                }
            }
            else
            {
                _log.Info("User not logged-in.");
            }
        }
        return accreditationReadViews;
    }

    private async Task<IEnumerable<AssociationReadView>> GetSchoolAssociations(int schoolId)
    {
        List<AssociationReadView> associationReadViews = new List<AssociationReadView>();
        using (var client = new HttpClient())
        {
            if (SessionToken != null)
            {
                try
                {
                    if (ValidateToken(client).IsSuccessStatusCode)
                    {
                        HttpResponseMessage response = await client.GetAsync("api/school/associationsbyschool/" + schoolId);
                        if (response.StatusCode == HttpStatusCode.OK)
                        {
                            List<Association> associations = await response.Content.ReadAsAsync<List<Association>>();
                            associationReadViews =
                                associations.Select(AssociationReadView.ShowAssociationView).ToList();
                        }
                        else
                        {
                            _log.Error("API Error Reason: " + response.ReasonPhrase);
                        }
                    }
                }
                catch (Exception sd)
                {
                    _log.Error("Error displaying Associations ", sd);
                }
            }
            else
            {
                _log.Info("User not logged-in.");
            }
        }
        return associationReadViews;
    }

    private async Task<IEnumerable<SchoolFacilityReadView>> GetSchoolFacilities(int schoolId)
    {
        IEnumerable<SchoolFacilityReadView> schoolFacilityReadViews = new List<SchoolFacilityReadView>();
        using (var client = new HttpClient())
        {
            if (SessionToken != null)
            {
                try
                {
                    if (ValidateToken(client).IsSuccessStatusCode)
                    {
                        HttpResponseMessage response = await client.GetAsync("api/school/facilities/" + schoolId);
                        if (response.StatusCode == HttpStatusCode.OK)
                        {
                            var schoolFacilities = await response.Content.ReadAsAsync<IEnumerable<SchoolFacility>>();
                            schoolFacilityReadViews = schoolFacilities.Select(SchoolFacilityReadView.ShowSchoolFacilityView);}
                        else
                        {
                            _log.Error("API Error Reason: " + response.ReasonPhrase);
                        }
                    }
                }
                catch (Exception sd)
                {
                    _log.Error("Error displaying School Facilities ", sd);
                }
            }
            else
            {
                _log.Info("User not logged-in.");
            }
        }
        return schoolFacilityReadViews;
    }

    private async Task<SchoolCurriculumReadView> GetSchoolCurriculum(int schoolId)
    {
        SchoolCurriculumReadView schoolCurriculumReadView = new SchoolCurriculumReadView();
        using (var client = new HttpClient())
        {
            if (SessionToken != null)
            {
                try
                {
                    if (ValidateToken(client).IsSuccessStatusCode)
                    {
                        HttpResponseMessage response = await client.GetAsync("api/school/getcurricula/" + schoolId);
                        if (response.StatusCode == HttpStatusCode.OK)
                        {
                            SchoolCurriculumInformation schoolCurriculumInformation =
                                await response.Content.ReadAsAsync<SchoolCurriculumInformation>();
                            schoolCurriculumReadView =
                                SchoolCurriculumReadView.ShowSchoolCurriculumView(schoolCurriculumInformation);
                        }
                        else
                        {
                            _log.Error("API Error Reason: " + response.ReasonPhrase);
                        }
                    }
                }
                catch (Exception sd)
                {
                    _log.Error("Error displaying Curriculum ", sd);
                }
            }
            else
            {
                _log.Info("User not logged-in.");
            }
        }
        return schoolCurriculumReadView;
    }

    private async Task<IEnumerable<MemberSchoolReadView>>  GetMemberSchools(int schoolId)
    {
        List<MemberSchoolReadView> memberSchoolReadViews = new List<MemberSchoolReadView>();
        using (var client = new HttpClient())
        {
            if (SessionToken != null)
            {
                try
                {
                    if (ValidateToken(client).IsSuccessStatusCode)
                    {
                        HttpResponseMessage response =
                            await client.GetAsync("api/school/memberschool/getallmemberschools/" + schoolId);

                        if (response.StatusCode == HttpStatusCode.OK)
                        {
                            IEnumerable<MemberSchool> memberSchools = await response.Content.ReadAsAsync<IEnumerable<MemberSchool>>();
                            memberSchoolReadViews = memberSchools.Select(MemberSchoolReadView.ShowMemberSchools).ToList();
                        }
                        else
                        {
                            _log.Error("API Error Reason: " + response.ReasonPhrase);
                        }
                    }
                }
                catch (Exception sd)
                {
                    _log.Error("Error displaying Member Schools for School with id: " + schoolId, sd);
                }
            }
            else
            {
                _log.Info("User not logged-in.");
            }
        }
        return memberSchoolReadViews;
    }

    private async Task<SchoolAddressReadView> GetSchoolAddress(int schoolId)
    {
        SchoolAddressReadView schoolAddress = null;
        using (var client = new HttpClient())
        {
            if (SessionToken != null)
            {
                try
                {
                    if (ValidateToken(client).IsSuccessStatusCode)
                    {
                        var response = await client.GetAsync("api/school/getaddressbyschoolid/" + schoolId);
                        if (response.StatusCode == HttpStatusCode.OK)
                        {
                            var responseContent = await response.Content.ReadAsAsync<SchoolAddress>();
                            schoolAddress = SchoolAddressReadView.ShowSchoolAddressView(responseContent);
                        }
                    }
                }
                catch (Exception sd)
                {
                    _log.Error("Error fetching School Address ", sd);
                }
            }
            else
            {
                _log.Info("User not logged-in.");
            }
        }
        return schoolAddress;
    }

    private async Task<SchoolReadView> GetSchool(int schoolId)
    {
        SchoolReadView school = null;
        using (var client = new HttpClient())
        {
            if (SessionToken != null)
            {
                try
                {
                    if (ValidateToken(client).IsSuccessStatusCode)
                    {
                        var response = await client.GetAsync("api/getallschools/" + schoolId);
                        if (response.StatusCode == HttpStatusCode.OK)
                        {
                            var responseContent = await response.Content.ReadAsAsync<School>();
                            school = SchoolReadView.ShowSchoolReadView(responseContent);
                        }
                    }
                }
                catch (Exception sd)
                {
                    _log.Error("Error getting school ", sd);
                }
            }
            else
            {
                _log.Info("User not logged-in.");
            }
        }
        return school;
    }

    private async Task<SchoolContactReadView> GetSchoolContact(int schoolId)
    {
        SchoolContactReadView schoolContact = null;
        using (var client = new HttpClient())
        {
            if (SessionToken != null)
            {
                try
                {
                    if (ValidateToken(client).IsSuccessStatusCode)
                    {
                        var response = await client.GetAsync("api/school/getcontactdetails/" + User.Identity.Name);
                        if (response.StatusCode == HttpStatusCode.OK)
                        {
                            var responseContent = await response.Content.ReadAsAsync<SchoolContact>();
                            schoolContact = SchoolContactReadView.ShowSchoolContactView(responseContent);
                        }
                    }
                }
                catch (Exception sd)
                {
                    _log.Error("Error getting school contact ", sd);
                }
            }
            else
            {
                _log.Info("User not logged-in.");
            }
        }
        return schoolContact;
    }



    /// <summary>
    /// Obtains the token from the Web API, the Session and the User Information
    /// </summary>
    /// <param name="client">Client object</param>
    /// <returns>Returns an HttpResponseMessage</returns>
    private HttpResponseMessage ValidateToken(HttpClient client)
    {

        client.BaseAddress = new Uri(baseUrl);
        client.DefaultRequestHeaders.Accept.Clear();
        client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));

        string accessToken = (string)SessionToken;
        client.DefaultRequestHeaders.Authorization
            = new AuthenticationHeaderValue("Bearer", accessToken);

        HttpResponseMessage responseToken = client.GetAsync(baseUrl + "api/Account/UserInfo").Result;
        return responseToken;
    }
}

and I also have an mvc controller action consuming this method

Public async Task<ActionResult> Details(int id)
{
    var model = await service.GetSchoolAggreagtedInfo(id).ConfigureAwait(false);
    return View(model)
}

I feel like I have too much async methods and which I'm afraid may cause more harm than good.

Is there a better way of doing all this async stuff (maybe a pattern), or is the code OK?

\$\endgroup\$
  • \$\begingroup\$ I pasted the code here and was about editing it when I lost internet connection. code updated anyway. It includes the class with all it's method. \$\endgroup\$ – pmbanugo Sep 24 '15 at 11:48
  • \$\begingroup\$ There is nothing wrong with using async everywhere for methods which are truly asynchronous, and your usage of it is easily readable. One complaint is fragility due to dependencies. You access HttpContext and AppSettings from a class that should have no awareness of them. This class would be a lot more robust if it used dependency injection. The constructor should accept an instance of IPrincipal, ILog, etc. \$\endgroup\$ – Brad M Sep 24 '15 at 15:07
3
\$\begingroup\$

Lots of very cool stuff in there.

But, there's also some bad stuff. You are significantly arrow-coded in some places.

One of the changes you should make is your guard clauses. Most explicitly, in this method:

private async Task<OrganizationContactReadView> GetOrganizationContact(int organizationId)
{
    OrganizationContactReadView organizationContact = null;

    using (var client = new HttpClient())
    {
        if (SessionToken != null)
        {
            try
            {
                if (ValidateToken(client).IsSuccessStatusCode)
                {
                    HttpResponseMessage response = await client.GetAsync("api/organization/contact/" + organizationId);
                    if (response.StatusCode == HttpStatusCode.OK)
                    {
                        var contentResult = await response.Content.ReadAsAsync<OrganizationContact>();
                        var organizationContactReadView = OrganizationContactReadView.MapFrom(contentResult);

                        organizationContact = organizationContactReadView;
                    }
                    else
                    {
                        _log.Error("API Error Reason: " + response.ReasonPhrase);
                    }
                }
            }
            catch (Exception sd)
            {
                _log.Error("Error displaying Organization Contact Information ", sd);
            }
        }
        else
        {
            _log.Info("User not logged-in.");
        }
    }

    return organizationContact;
}

Let's look at what we can improve here.

First, we notice that if SessionToken != null then things cannot really happen here. So, let's move that to the top of the file.

if (SessionToken == null)
{
    _log.Info("User not logged-in.");
    // Throw an exception, maybe? If not, return null;
}

So, now we get rid of some of the arrow.

Next, I would do something like:

using (var client = new HttpClient())
{
    if (!ValidateToken(client).IsSuccessStatusCode)
    {
        // Throw an exception most likely, or return null;
    }

    // We know at this point two things are true: the `SessionToken` is good, and so is `ValidateToken(client)`.

    try
    {
        // This string format on the next line is C#6.0.
        HttpResponseMessage response = await client.GetAsync($"api/organization/contact/{organizationId}");

        if (response.StatusCode != HttpStatusCode.OK)
        {
            _log.Error($"API Error Reason: {response.ReasonPhrase}");
            // Throw an exception, return null, whatever.
        }

        var contextResult = await response.Content.ReadAsAsync<OrganizationContact>();
        var organizationContactReadView = OrganizationContactReadView.MapFrom(contentResult);

        organizationContact = organizationContactReadView;
    }
    catch (Exception ex)
    {
        _log.Error("Error displaying Organization Contact Information ", ex);
    }
}

Obviously both pieces need put together.

You can do similar things with your methods that smell like arrow-code. (This is a pun on Code-Smell, if anyone missed it.)


If you are going to prefix one field name with an underscore, you should prefix all field names with underscores.

I.e. refactor private static string baseUrl to private static string _baseUrl.


I would recommend creating an interface to hold common elements between the OrganizationContactReadView, OrganizationAssociationReadView and OrganizationReadView.

This will allow you to abstract out a little further, and create actual responsibility separations.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.