locked
Programming wisdom needed RRS feed

  • Question

  • User-1104215994 posted

    Hi,

    I have <g class="gr_ gr_26 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar multiReplace" id="26" data-gr-id="26">a asp</g>.net web <g class="gr_ gr_38 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="38" data-gr-id="38">api</g>. Here is how it works;

    • <g class="gr_ gr_204 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar only-ins doubleReplace replaceWithoutSep" id="204" data-gr-id="204">client</g> sends a request to method A in my web <g class="gr_ gr_170 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="170" data-gr-id="170">api</g> and my web <g class="gr_ gr_436 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="436" data-gr-id="436">api</g> either <g class="gr_ gr_539 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar multiReplace" id="539" data-gr-id="539">queries</g> a table if there is no result a 3rd party web <g class="gr_ gr_344 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="344" data-gr-id="344">api</g> method A is called.
    • In method A I am saving request and response into <g class="gr_ gr_895 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar only-ins replaceWithoutSep" id="895" data-gr-id="895">database</g> table (response may change due to the result of the querying the table

    Then there is method B, logic is the same with method A.

    • <g class="gr_ gr_988 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar only-ins doubleReplace replaceWithoutSep" id="988" data-gr-id="988">client</g> sends a request (pretty much the same data) to method B in my web <g class="gr_ gr_981 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="981" data-gr-id="981">api</g> and my web <g class="gr_ gr_982 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="982" data-gr-id="982">api</g> either queries a table if there is no result a 3rd party web <g class="gr_ gr_983 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="983" data-gr-id="983">api</g> method B is called.
    • In method B I am saving request and response into <g class="gr_ gr_1140 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar only-ins replaceWithoutSep" id="1140" data-gr-id="1140">database</g> table (response may change due to the result of the querying the table

    Now <g class="gr_ gr_1162 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar only-ins replaceWithoutSep" id="1162" data-gr-id="1162">client</g> wants me to combine these 2 methods A and B. What would you suggest? Methods are already fat, I know I need to refactor them but for time being I just want to find a way to make this work.

    Here is my controller and 2 methods:

     [System.Web.Http.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)
            {
                #region Is referenceId Unique?
               
                if (await Utilities.IsUnique(context, initiate) > 0)
                {
                    ModelState.AddModelError("referenceId", "This referenceId has used!");
                }
                
                #endregion
                if (!ModelState.IsValid)
                {
                    return BadRequest(ModelState);
                }
    
               
                #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)
                    ).ToListAsync();
                //If we have exact number of games in our database, mark them!
                if (gameBankResult.Count() != 0 && gameBankResult.Count() >= initiate.quantity)
                {
                    for (var index = 0; index < initiate.quantity; index++)
                    {
                        var item = gameBankResult[index];
                        item.referenceId = initiate.referenceId;
                        context.Entry(item).State = System.Data.Entity.EntityState.Modified;
                    }
    
    
                    //Update marked games
                    await context.SaveChangesAsync();
                    
                    //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 = null,
                        })).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.referenceId = Guid.NewGuid().ToString();
                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 3rd party
                var response =
                     await httpClient.PostAsync("https://test.com/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);
                        //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
            
            [HttpPost, Route("confirmation")]
            public async Task<IHttpActionResult> PostConfirmation(ConfirmRequest confirm)
            {
                if (!ModelState.IsValid)
                {
                    return BadRequest(ModelState);
                }
    
              
                #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);
               //call 3rd party
               var response =
                    await httpClient.PostAsync("https://test.com/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);
            }
    ....

    Sunday, June 23, 2019 7:38 AM

Answers

  • User71929859 posted

    Not sure why you are doing a dependency injection for unit of work in service classes. Unit of work suppose to do it's work in repository level.

    cenk1536

    public BusinessEntities.GameRequest GetGameById(int gameId)
            {
                var game = _unitOfWork.GameRepository.GetByID(gameId);
                // write my logic and calling 3rd party web api
                return game;
            }

    Yes, this is pretty much what I'm talking about. 

    Then call the service class methods from your controller.

    Something like below

    private readonly IGameService _gameService;
    
    public GameController(IGameService gameService)
    {
          _gameService = gameService;
    }
    
    [HttpPost]
    public void DoBothInOneAPI() //This is not a good naming convention BTW
    {
          _gameService.Initiate();
          _gameService.Confirm(), 
    }

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Friday, June 28, 2019 5:16 AM

All replies

  • User1120430333 posted

    The ASP.NET WebAPI is using the ASP.NET MVC design pattern, which has a Models folder. Now keeping in mind that your usage of the WebAPI has no MVC UI views, it could have the views too, but the WebAPI doesn't and that's 99% of the time, understand the information that is being discussed about the Models folder and the controller  and how to use them in implementing seperation of concerns.

    https://docs.microsoft.com/en-us/aspnet/mvc/overview/older-versions-1/overview/understanding-models-views-and-controllers-cs

    <copied>

    Understanding Models

    We have discussed controllers and we have discussed views. The last topic that we need to discuss is models. What is an MVC model?

    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. For example, if you are using the Microsoft Entity Framework to access your database, then you would create your Entity Framework classes (your .edmx file) in the Models folder.

    A view should contain only logic related to generating the user interface. A controller should only contain the bare minimum of logic required to return the right view or redirect the user to another action (flow control). Everything else should be contained in the model.

    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://en.wikipedia.org/wiki/Separation_of_concerns

    IMO, there is no reason for you to be having business logic and database logic in the controller. All of the logic you have can be moved to one class in the Models folder that implements an Interface that can be placed into an IoC, dependency injected into the controller and a method or methods of the injected object into the controller can be acted upon by controller action methods.

    Two ways to implement SoC are the usage of Visual Stuido project folder, like the Models folder or you make your own folder, which is called namespace seperation, and the other way is to create a classlib project, make a class or classes in the classlib project, move all the logic into a class or classes that implement an Interface in the classlib project that can be referenced by the WebAPI project and the object or objects in the classlib project DI into the WebAPI controller.

    You have more code in two action methods of the WebAPI controller than I am showing in the entire controller, and it's becuase SoC is being implemented, which can be easily unit tested due to usage of an Interface implemented on the class in the classlib project called the Data Access Layer that is being DI into the WebAPI controller. 

    So you could have all the logic in one class if you choose to do so, and I don't see it as some refactoring attempt, but it's rather, simply move all of the logic out of the controller.:)  

    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
        {
            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);
            }
        }
    }

    Sunday, June 23, 2019 12:42 PM
  • User71929859 posted

    Now client wants me to combine these 2 methods A and B. What would you suggest? Methods are already fat, I know I need to refactor them but for time being I just want to find a way to make this work.

    Proper way of doing this would be move the business logic in to Service layers, move the data access to repositories and by using interfaces, do dependency injection to call to those methods from your controller. Use some IoC container like Ninject or Castle Windsor to do that.

    To answer your question to the point, you can use HttpWebClient class to make a web request from your 1st API endpoint. But keep in mind that this is a very bad practise and I don't recommend it at all!

    Thursday, June 27, 2019 5:18 AM
  • User-1104215994 posted

    To answer your question to the point, you can use HttpWebClient class to make a web request from your 1st API endpoint. But keep in mind that this is a very bad <g class="gr_ gr_5 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling" id="5" data-gr-id="5">practise</g> and I don't recommend it at all!

    I am calling as follows. If it is a bad <g class="gr_ gr_170 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling" id="170" data-gr-id="170">practise</g>, how should I call?

    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.com/purchaseinitiation",content);

    Thursday, June 27, 2019 5:38 AM
  • User71929859 posted

    I am calling as follows. If it is a bad practise, how should I call?

    There's no good way to call it other than the way you are already doing. What I'm saying is I smell a bad code when an internal call is made to a Web API endpoint from another end point. That basically tells you there's a serious design defect.

    Thursday, June 27, 2019 5:51 AM
  • User-1104215994 posted

    What I'm saying is I smell a bad code when an internal call is made to a Web API endpoint from another <g class="gr_ gr_5 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del" id="5" data-gr-id="5">end point</g>. That basically tells you there's a serious design defect.

    Is there a way to make design better?

    Thursday, June 27, 2019 6:40 AM
  • User71929859 posted

    Is there a way to make design better?

    Yes, move the logic to service layer. You can have service classes for the logic. And call those service classes by doing DI to the controller. In that way, you can call the two methods relevant to both initiate and confirmation logics inside one controller action. 

    Thursday, June 27, 2019 6:30 PM
  • User-1104215994 posted

    thank you, can you check my Unity.Aspnet.<g class="gr_ gr_7 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="7" data-gr-id="7">Webapi</g> question?

    Do you mean something like this? Just moving the code that I posted into the sample below?

    public class GameServices : IGameServices
        {
            private readonly UnitOfWork _unitOfWork;
    
            /// <summary>
            /// Public constructor.
            /// </summary>
            public GameServices(UnitOfWork unitOfWork)
            {
                _unitOfWork = unitOfWork;
            }
            /// <summary>
            /// Fetches product details by id
            /// </summary>
            /// <param name="gameId"></param>
            /// <returns></returns>
            public BusinessEntities.GameRequest GetGameById(int gameId)
            {
                var game = _unitOfWork.GameRepository.GetByID(gameId);
                // write my logic and calling 3rd party web api
                return game;
            }

    Thursday, June 27, 2019 6:36 PM
  • User475983607 posted

    You've asked this question several times across several threads and we all recommended basically the same thing.   Repositories and UoW patterns have nothing to do with the issue.  Nor does an IoC container.

    The main problem is calling a 3rd party service twice within a single Web API request.  You still have not explained if this is possible given how the 3rd party service works.

    Keep in mind, that once you get basic functionality working (calling the service twice in a single method) then you can refactor the code.  That means implement Unity, the Repository pattern, and UoW.

    Thursday, June 27, 2019 7:59 PM
  • User71929859 posted

    Not sure why you are doing a dependency injection for unit of work in service classes. Unit of work suppose to do it's work in repository level.

    cenk1536

    public BusinessEntities.GameRequest GetGameById(int gameId)
            {
                var game = _unitOfWork.GameRepository.GetByID(gameId);
                // write my logic and calling 3rd party web api
                return game;
            }

    Yes, this is pretty much what I'm talking about. 

    Then call the service class methods from your controller.

    Something like below

    private readonly IGameService _gameService;
    
    public GameController(IGameService gameService)
    {
          _gameService = gameService;
    }
    
    [HttpPost]
    public void DoBothInOneAPI() //This is not a good naming convention BTW
    {
          _gameService.Initiate();
          _gameService.Confirm(), 
    }

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Friday, June 28, 2019 5:16 AM