Code review needed

Question

User-1104215994 posted

Hello guys,

Here is my asp.net web api controller. I would be glad if you can review and improve my code in terms of maintainability, refactoring, simplicity, incorrect/incomplete usage and etc. Please don't be cruel :)

 [RoutePrefix("api/v2/pin")]
    public class InitiatesController : ApiController
    {
        private readonly EPINMiddleWareAPIContext context;

        public InitiatesController(EPINMiddleWareAPIContext context)
        {
            this.context = context;
        }


        // POST: api/Game
        //[RequireHttps] For Prod Only
        [HttpPost, Route("initiation")]
        public async Task<IHttpActionResult> PostInitiate(InitiateRequest initiate)
        {
            if (!ModelState.IsValid)
            {
                return BadRequest(ModelState);
            }


            #region Validate Company
            var responseMsg = new HttpResponseMessage();
            var isUsernamePasswordValid = false;
            
            //Find Company in our database
            var companyCheck =
                await (context.Companies.Where(p => p.customerID == initiate.customerID)).SingleOrDefaultAsync();
            //Check company password in our database
            if (companyCheck != null)
                isUsernamePasswordValid = Utilities.VerifyPassword(initiate.password, companyCheck.hash, companyCheck.salt);
            //Send UnAuthorized if we don't know the company
            if (!isUsernamePasswordValid)
            {
                // if credentials are not valid send unauthorized status code in response
                responseMsg.StatusCode = HttpStatusCode.Unauthorized;
                IHttpActionResult rMessage = ResponseMessage(responseMsg);
                return rMessage;
            }

            #endregion

            #region Check Code Bank database
            //Check Games in our database - all or nothing
            var gameBankResult = await (context.GameBanks.Where(g => g.productCode == initiate.productCode)
                .Where(l => l.referenceId == null)
                .Take(initiate.quantity)).ToListAsync();
            //If we have games in our database, mark them!
            if (gameBankResult.Count() != 0)
            {
                foreach (var item in gameBankResult)
                {
                    item.referenceId = initiate.referenceId;
                    context.Entry(item).State = System.Data.Entity.EntityState.Modified;
                }

                
                //Update marked games
                await context.SaveChangesAsync();
                //Create token
                var token = Utilities.createToken(companyCheck.userName);
                //Return marked games in our database
                var gameBankResultVM = await (context.GameBanks.Where(g => g.productCode == initiate.productCode)
                    .Where(l => l.referenceId == initiate.referenceId)
                    .Take(initiate.quantity)
                    .Select(g => new GameBankInitiationResponseVM()
                    {
                        referenceId = g.referenceId,
                        productCode = g.productCode,
                        quantity = initiate.quantity,
                        initiationResultCode = g.initiationResultCode,
                        validatedToken = g.validatedToken,
                        currency = g.currency,
                        estimateUnitPrice = g.estimateUnitPrice,
                        version = g.version,
                        signature = g.signature,
                        ApplicationCode = g.ApplicationCode,
                        companyToken = token,
                    })).ToListAsync();

                
                var resultObj = gameBankResultVM[0];
                
                var initiateResponse = JsonConvert.SerializeObject(resultObj);
                var responseDB = JsonConvert.DeserializeObject<InitiateResponse>(initiateResponse);
                //Adding Response into database
                context.InitiateResponses.Add(responseDB);
                await context.SaveChangesAsync();
                return Ok(resultObj);

            }


            #endregion

            // Add Signature
            initiate = Utilities.CreateSignature(initiate);


            #region Insert Request into DB
            //Adding Request into database
            context.InitiateRequests.Add(initiate);
            await context.SaveChangesAsync();
            #endregion

            
            #region Call Game
            var httpClient = new HttpClient();
            //Convert request
            var keyValues = initiate.ToKeyValue();
            var content = new FormUrlEncodedContent(keyValues);
            //Call Game Sultan
            var response =
                 await httpClient.PostAsync("https://test/purchaseinitiation",content);

            #endregion
            //Read response
            var htmlResponse = await response.Content.ReadAsStringAsync();
            switch (response.StatusCode)
            {
                case HttpStatusCode.NotFound:
                {
                    return NotFound();
                }

                case HttpStatusCode.InternalServerError:
                {
                    return InternalServerError();
                }

                case HttpStatusCode.OK:
                {
                   
                    var initiateResponse = JsonConvert.DeserializeObject<InitiateResponse>(htmlResponse);
                    //Create Token if the result is SUCCESS
                    if (initiateResponse.initiationResultCode == "00")
                    {
                        //Create TOKEN and set
                        initiateResponse.companyToken = Utilities.createToken(companyCheck.userName); 
                    }
                        
                    //Adding Response into database
                    context.InitiateResponses.Add(initiateResponse);
                    await context.SaveChangesAsync();
                    return Ok(initiateResponse);
                }

                case HttpStatusCode.BadRequest:
                {
                    return BadRequest(htmlResponse);
                }

                case HttpStatusCode.Unauthorized:
                {
                    return Unauthorized();
                }

                case HttpStatusCode.RequestTimeout:
                {
                    return InternalServerError();
                }

                default:
                {
                    htmlResponse = await response.Content.ReadAsStringAsync();
                    break;
                }
            }

            return Ok(htmlResponse);

        }


        // POST: api/Game
        //[RequireHttps] For Prod Only
        [Authorize]
        [HttpPost, Route("confirmation")]
        public async Task<IHttpActionResult> PostConfirmation(ConfirmRequest confirm)
        {
            if (!ModelState.IsValid)
            {
                return BadRequest(ModelState);
            }

            #region Validate Company
            var responseMsg = new HttpResponseMessage();
            var isUsernamePasswordValid = false;

            //Find Company in our database
            var companyCheck =
                await (context.Companies.Where(p => p.customerID == confirm.customerID)).SingleOrDefaultAsync();
            //Check company password in our database
            if (companyCheck != null)
                isUsernamePasswordValid = Utilities.VerifyPassword(confirm.password, companyCheck.hash, companyCheck.salt);
            //Send UnAuthorized if we don't know the company
            if (!isUsernamePasswordValid)
            {
                // if credentials are not valid send unauthorized status code in response
                responseMsg.StatusCode = HttpStatusCode.Unauthorized;
                IHttpActionResult rMessage = ResponseMessage(responseMsg);
                return rMessage;
            }

            #endregion

            #region Check Code Bank database
            //Return marked games in our database
            var gameBankResultVM = await (context.GameBanks
                .Where(l => l.referenceId == confirm.referenceId)
                .Select(g => new GameBankConfirmResponseVM()
                {
                    referenceId = g.referenceId,
                    paymentId = null,
                    productCode = g.productCode,
                    quantity = g.quantity,
                    deliveredQuantity = g.quantity,
                    currency = g.currency,
                    version = g.version,
                    signature = g.signature,
                    ApplicationCode = g.ApplicationCode,
                    productDescription = g.productDescription,
                    unitPrice = g.unitPrice,
                    totalPrice = g.totalPrice,
                    totalPayablePrice = g.totalPrice,
                    coupons = g.coupons.Select(c => new GameCouponBankVM()
                    {
                        Pin = c.Pin,
                        Serial = c.Serial,
                        expiryDate = c.expiryDate
                    }).ToList()
                })).ToListAsync();

            //If we have games in our database, select them!
            if (gameBankResultVM.Count() != 0)
            {
                if (gameBankResultVM.Count <= 1)
                {
                    //Adding Response into database
                    var responseNew = JsonConvert.SerializeObject(gameBankResultVM[0]);
                    var confirmResponse = JsonConvert.DeserializeObject<ConfirmResponse>(responseNew);
                    context.ConfirmResponses.Add(confirmResponse);
                    await context.SaveChangesAsync();

                    return Ok(gameBankResultVM[0]);
                }
                else if(gameBankResultVM.Count > 1)
                {
                    var gameResult = new GameBankConfirmResponseVM();
                    var price = 0.0;
                    var quantity = 0;

                    foreach (var item in gameBankResultVM)
                    {
                        price = price + item.unitPrice;
                        quantity = quantity + 1;
                        foreach (var coupons in item.coupons)
                        {
                            var gameCouponResult = new GameCouponBankVM
                            {
                                expiryDate = coupons.expiryDate,
                                Pin = coupons.Pin,
                                Serial = coupons.Serial
                            };
                            //Add coupon values
                            gameResult.coupons.Add(gameCouponResult);
                        }

                    }
                    //Set summed/counted values
                    gameResult.referenceId = gameBankResultVM[0].referenceId;
                    gameResult.paymentId = gameBankResultVM[0].paymentId;
                    gameResult.productCode = gameBankResultVM[0].productCode;
                    gameResult.quantity = quantity;
                    gameResult.deliveredQuantity = quantity;
                    gameResult.currency = gameBankResultVM[0].currency;
                    gameResult.unitPrice = gameBankResultVM[0].unitPrice;
                    gameResult.totalPrice = price;
                    gameResult.purchaseStatusDate = gameBankResultVM[0].purchaseStatusDate;
                    gameResult.productDescription = gameBankResultVM[0].productDescription;
                    gameResult.totalPayablePrice = price;
                    gameResult.version = gameBankResultVM[0].version;
                    gameResult.signature = null;
                    gameResult.ApplicationCode = null;



                    //Adding Response into database
                    var responseNew = JsonConvert.SerializeObject(gameResult);
                    var confirmResponse = JsonConvert.DeserializeObject<ConfirmResponse>(responseNew);
                    context.ConfirmResponses.Add(confirmResponse);
                    await context.SaveChangesAsync();

                    return Ok(gameResult);
                }
            }

            #endregion

            // Add Signature
            confirm = Utilities.CreateSignature(confirm);
            // Save Request into Table
            context.ConfirmRequests.Add(confirm);
            await context.SaveChangesAsync();

            var httpClient = new HttpClient();

            var keyValues = confirm.ToKeyValue();
            var content = new FormUrlEncodedContent(keyValues);

           var response =
                await httpClient.PostAsync("https://test/purchaseconfirmation", content);


           var htmlResponse = await response.Content.ReadAsStringAsync();
            switch (response.StatusCode)
            {
                case HttpStatusCode.NotFound:
                    return NotFound();

                case HttpStatusCode.InternalServerError:
                    return InternalServerError();

                case HttpStatusCode.OK:

                    //Adding Response into database
                    var newResult = JObject.Parse(htmlResponse);
                    var confirmResponse = JsonConvert.DeserializeObject<ConfirmResponse>(newResult.ToString());
                    context.ConfirmResponses.Add(confirmResponse);
                    await context.SaveChangesAsync();
                    return Ok(newResult);

                case HttpStatusCode.BadRequest:
                    return BadRequest();

                case HttpStatusCode.Unauthorized:
                    return Unauthorized();

                case HttpStatusCode.RequestTimeout:
                    return InternalServerError();

                default:
                    htmlResponse = await response.Content.ReadAsStringAsync();
                    break;
            }

            return Ok(htmlResponse);
        }


        [HttpPatch, Route("reconInitiation")]
        public async Task<IHttpActionResult> PatchReconciliationInitiation(Recon recon)
        {

            if (!ModelState.IsValid)
            {
                return BadRequest(ModelState);
            }

            #region Validate Company
            var responseMsg = new HttpResponseMessage();
            var isUsernamePasswordValid = false;

            //Company password check
            var companyCheck =
                await (context.Companies.Where(p => p.customerID == recon.customerID)).SingleOrDefaultAsync();
            if (companyCheck != null)
                isUsernamePasswordValid = Utilities.VerifyPassword(recon.password, companyCheck.hash, companyCheck.salt);
            if (!isUsernamePasswordValid)
            {
                // if credentials are not valid send unauthorized status code in response
                responseMsg.StatusCode = HttpStatusCode.Unauthorized;
                IHttpActionResult rMessage = ResponseMessage(responseMsg);
                return rMessage;
            }

            #endregion
            //Projection
            var reconInitiateVm = new ReconInitiationVM();
            //Create TOKEN and set
            reconInitiateVm.companyToken = Utilities.createToken(companyCheck.userName);
           
            return Ok(reconInitiateVm);
        }

        [Authorize]
        [HttpPatch, Route("reconConfirmation")]
        public async Task<IHttpActionResult> PatchReconciliationConfirmation(Recon recon)
        {

            if (!ModelState.IsValid)
            {
                return BadRequest(ModelState);
            }

            #region Validate Company
            var responseMsg = new HttpResponseMessage();
            var isUsernamePasswordValid = false;

            //Company password check
            var companyCheck =
                await (context.Companies.Where(p => p.customerID == recon.customerID)).SingleOrDefaultAsync();
            if (companyCheck != null)
                isUsernamePasswordValid = Utilities.VerifyPassword(recon.password, companyCheck.hash, companyCheck.salt);
            if (!isUsernamePasswordValid)
            {
                // if credentials are not valid send unauthorized status code in response
                responseMsg.StatusCode = HttpStatusCode.Unauthorized;
                IHttpActionResult rMessage = ResponseMessage(responseMsg);
                return rMessage;
            }

            #endregion

            #region Check Our Database
            DateTime? today = DateTime.Today;
            var reconResult = await (context.GameBanks.Where(g => g.used == 0)
                    .Where(l => l.referenceId != null)
                    .Where(t => System.Data.Entity.DbFunctions.TruncateTime(t.responseDateTime) == today)
                ).ToListAsync();

            if (reconResult.Count() != 0)
            {

                foreach (var item in reconResult)
                {
                    foreach (var reference in recon.referenceId)
                    {

                        if (item.referenceId == reference)
                        {
                                item.used = 1;
                                context.Entry(item).State = System.Data.Entity.EntityState.Modified;
                        }

                    }
                }


                await context.SaveChangesAsync();

            }

            #endregion

            #region Check ConfirmResponse - Razer
            
            var reconResultRazer = await (context.ConfirmResponses.Where(g => g.used == 0)
                    .Where(l => l.referenceId != null)
                    .Where(t => System.Data.Entity.DbFunctions.TruncateTime(t.purchaseStatusDate) == today)
                ).ToListAsync();

            if (reconResultRazer.Count() != 0)
            {

                foreach (var item in reconResultRazer)
                {
                    foreach (var reference in recon.referenceId)
                    {

                        if (item.referenceId == reference)
                        {
                            item.used = 1;
                            context.Entry(item).State = System.Data.Entity.EntityState.Modified;
                        }

                    }
                }


                await context.SaveChangesAsync();

            }
            #endregion


            return StatusCode(HttpStatusCode.NoContent);


        }

        protected override void Dispose(bool disposing)
        {
            context.Dispose();
            base.Dispose(disposing);
        }

       
    }

Answer 1

User1120430333 posted

I treat the WebAPI controller like I treat the MVC controller where SoC is implemented and the controller doesn't have  a whole lot of code in it, a thin controller.  It is just my view of it my take on things. 

https://en.wikipedia.org/wiki/Separation_of_concerns

https://code-maze.com/aspnetcore-webapi-best-practices/

<copied>

Project Organization

We should always try to split our application into smaller projects. That way we are getting the best project organization and separation of concerns (SoC). The business logic related to our entities, contracts, accessing the database, logging messages or sending an email message should always be in a separate .NET Core Class Library project.

<end>

Although the controller I am using is not doing all that the controller you have presented, with your controller I would implement SoC and segregate the logic.

using DAL;
using Entities;
using Microsoft.AspNetCore.Mvc;
using System.Collections.Generic;

namespace ProgMgmntCore2Api.Controllers
{
    [Produces("application/json")]
    [Route("api/[controller]")]
    [ApiController]

    public class TaskController : ControllerBase, ITaskController
    {
        private readonly IDaoTask _daoTask;

        public TaskController(IDaoTask daoTask)
        {
            _daoTask = daoTask;
        }

        [HttpGet]
        [Route("GetTaskById")]
        public DtoTask GetTaskById(int id)
        {
            return _daoTask.GetTaskById(id);
        }

        [HttpGet]
        [Route("GetTasksByProjId")]
        public List<DtoTask> GetTasksByProjectId(int id)
        {
            return _daoTask.GetTasksByProjectId(id);
        }

        [HttpPost]
        [Route("CreateTask")]
        public void CreateTask(DtoTask dto)
        {
            _daoTask.CreateTask(dto);
        }

        [HttpPost]
        [Route("UpdateTask")]
        public void UpdateTask(DtoTask dto)
        {
            _daoTask.UpdateTask(dto);
        }

        [HttpPost]
        [Route("DeleteTask")]
        public void DeleteTask(DtoId dto)
        {
            _daoTask.DeleteTask(dto.Id);
        }
    }
}

Answer 2

User-1104215994 posted

I guess I need to deal with duplicate code. How can I make a function of <g class="gr_ gr_57 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar multiReplace" id="57" data-gr-id="57">validate</g> company region in <g class="gr_ gr_73 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar multiReplace" id="73" data-gr-id="73">Utilities</g> class like create signature method?

Answer 3

User1120430333 posted

I guess I need to deal with duplicate code. How can I make a function of validate company region in Utilities class like create signature method?

You could put the Utilities class in the Models folder, instance the object and use methods/behavior of the Utilities object from the controller 

I view the Models folder in WebAPI like I view it in MVC. Again, it's how I view it.

https://www.tutorialsteacher.com/webapi/what-is-web-api

<copied>

ASP.NET Web API

The ASP.NET Web API is an extensible framework for building HTTP based services that can be accessed in different applications on different platforms such as web, windows, mobile etc. It works more or less the same way as ASP.NET MVC web application except that it sends data as a response instead of html view. It is like a webservice or WCF service but the exception is that it only supports HTTP protocol.

<end>

https://www.c-sharpcorner.com/UploadFile/56fb14/understanding-separation-of-concern-and-Asp-Net-mvc/

<copied>

An MVC model contains all of your application logic that is not contained in a view or a controller. The model should contain all of your application business logic, validation logic, and database access logic.

In general, you should strive for fat models and skinny controllers. Your controller methods should contain only a few lines of code. If a controller action gets too fat, then you should consider moving the logic out to a new class in the Models folder.

<end>

https://www.upgrad.com/blog/mvc-architecture-in-java/

<copied>

MVC architectural pattern follows an elementary idea – we must separate the responsibilities in any application on the following basis:

1) Model: Handles data and business logic.
2) View: Presents the data to the user whenever asked for.
3) Controller: Entertains user requests and fetch necessary resources.

First, the “pens_controller.php” handles the user request (1) as a GET or POST request. We can also have an “index.php” which is the central controller which will call the “pens_controller” whenever needed.

The controller then examines the request and the parameters and calls the required model – in this case, “pens_model.php”. The controller asks the model to return the list of available pens.

Now, the model searches the database for the necessary information , applies logics if necessary, and returns the data to the controller.
The controller then picks an appropriate view (5) and presents the data (6 and 7). If a request comes from a handheld device, a view suitable for it will be used, or if the user has a particular theme selected, its view will be picked – and so on.
<end>