2
\$\begingroup\$

I have an ASP.NET CORE web API. I have a method that queries two tables: Delivery and DeliveryAssignment.

They have temporary tables DeliveryAssignmentDetail which contain deliveryId and AssignmentId.

Input: UserId (When user logs into the app and then it through gen id)

Output: List DeliveryAssignment Object and each of Delivery object in model

DeliveryAssignment

My method in code:

using Api.App.Application.Query.Model.Delivery; using Api.App.Application.Query.Model.DeliveryAssignment; using Hacom.Core.Extensions; using Hacom.Service.Security; using Microsoft.EntityFrameworkCore; using Minio.DataModel; using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using System.Text; using System.Threading.Tasks; namespace Api.App.Application.Query.Handler.DeliveryAssigment { public class GetTblDeliveryAssignmentToDetailAssginmentForAppQuery : IQueryBase<IEnumerable<GetDeliveryAssignmentToDetailAssignmentForAppModel>> { } public class GetTblDeliveryAssignmentToDetailAssginmentForAppQueryHandler : IRequestBaseHandler<GetTblDeliveryAssignmentToDetailAssginmentForAppQuery, IEnumerable<GetDeliveryAssignmentToDetailAssignmentForAppModel>> { private readonly IRepositoryService _repositoryService; private readonly IMapper _mapper; private readonly IHybridCachingManager _cacheExtension; private readonly IAuthorizeExtension _authorizeExtension; public GetTblDeliveryAssignmentToDetailAssginmentForAppQueryHandler(IMapper mapper, IHybridCachingManager cacheExtension, IAuthorizeExtension autho) { _repositoryService = EngineContext.Current.Resolve<IRepositoryService>(DataConnectionHelper.ConnectionStringNames.Pos) ?? throw new ArgumentNullException(nameof(_repositoryService)); _mapper = mapper ?? throw new ArgumentNullException(nameof(mapper)); _cacheExtension = cacheExtension ?? throw new ArgumentNullException(nameof(mapper)); _authorizeExtension = autho ?? throw new ArgumentNullException(nameof(autho)); } public async Task<IEnumerable<GetDeliveryAssignmentToDetailAssignmentForAppModel>> Handle(GetTblDeliveryAssignmentToDetailAssginmentForAppQuery request, CancellationToken cancellationToken) { if (request is null) throw new BaseException("Yêu cầu không hợp lệ"); // Query user var userId = (await _authorizeExtension.GetUser()).Empid; // Query DeliveryAssignment var deliveryAssignment = _repositoryService.WhereTracking<TblDeliveryAssignment>(src => src.Employee1 == userId || src.Employee2 == userId); var deliveryAssignmentIds = deliveryAssignment.Select(src => src.Id); // Query Delivery var delivery = _repositoryService.Table<TblDelivery>(); var deliveryIds = delivery.Select(src => src.Id); // Query Detail var detail = await _repositoryService.Table<TblDeliveryAssignmentDetail>().Where(src => deliveryIds.Contains((long)src.DeliveryId) && deliveryAssignmentIds.Contains((long)src.AssignmentId)).ToListAsync(); if (!detail.AnyList()) throw new BaseException("Không có dữ liệu"); var res = detail.Select(src => { var deliveryAssignmentEntity = deliveryAssignment.FirstOrDefault(da => da.Id == src.AssignmentId); var mappedDeliveryAssignment = _mapper.Map<GetDeliveryAssignmentToDetailAssignmentForAppModel>(deliveryAssignmentEntity); var mappedDeliveries = delivery.Where(d => d.Id == src.DeliveryId) .Select(d => _mapper.Map<TblDeliveryModel>(d)); if (mappedDeliveryAssignment != null) { mappedDeliveryAssignment.tblDeliveryModels = mappedDeliveries; } return mappedDeliveryAssignment; }); return res; } } } 

Model

using Api.App.Application.Query.Model.Delivery; using System; using System.Collections.Generic; using System.Linq; using System.Text; using System.Threading.Tasks; namespace Api.App.Application.Query.Model.DeliveryAssignment { public class GetDeliveryAssignmentToDetailAssignmentForAppModel { public long Id { get; set; } public string Code { get; set; } public DateTime? AssignmentDate { get; set; } public DateTime? FromExpectedDate { get; set; } public DateTime? ToExpectedDate { get; set; } public long? DeliveryWorkId { get; set; } public long? Employee1 { get; set; } public long? Employee2 { get; set; } public bool? MainResponsibility1 { get; set; } public bool? MainResponsibility2 { get; set; } public bool? Payroll1 { get; set; } public bool? Payroll2 { get; set; } public string VehicleCode { get; set; } public long? DriverId { get; set; } public string PartnerEmployeeName { get; set; } public string Description { get; set; } public IEnumerable<TblDeliveryModel> tblDeliveryModels { get; set; } } } 
\$\endgroup\$

    1 Answer 1

    1
    \$\begingroup\$

    consistent errors

     public GetTblDeliveryAssignmentToDetailAssginmentForAppQueryHandler( ... ) { ... ?? throw new ArgumentNullException(nameof(_repositoryService)); ... ?? throw new ArgumentNullException(nameof(mapper)); ... ?? throw new ArgumentNullException(nameof(mapper)); ... ?? throw new ArgumentNullException(nameof(autho)); } 

    I don't understand what's going on here:

     if (request is null) throw new BaseException("Yêu cầu không hợp lệ"); 

    Wouldn't we like to tell the caller it's a Null Argument? Also, always prefer to throw something more specific than Base, so caller can focus on a specific error that it knows how to recover from.

    Similarly for the subsequent "No data" error. Though there, it's not clear why we raise an error instead of simply forging ahead with an empty container, to return null. If some stakeholder's specification spelled out such a requirement, the Review Context didn't mention it.

    unused argument

    Why do we accept a cancellationToken, only to ignore it? Did your IDE or linter not highlight this detail? Is there some Public API, outside our control, which requires that second parameter?

    synchronous

     var userId = (await _authorizeExtension.GetUser()).Empid; 

    Do we really need to await the result? Couldn't WhereTracking deal with a future just fine, saving a roundtrip? Maybe this is leftover from debugging?

    silly name

     public bool? MainResponsibility1 { get; set; } public bool? MainResponsibility2 { get; set; } 

    The first one makes sense.

    By the time we get to the second one, it has become clear that this is not your single "main" responsibility, but rather it is just an element drawn from some set of responsibilities.

    Also, if a unique Code is externally assigned to the record, it's unclear why we need to invent a synthetic Id for it.

    \$\endgroup\$
    1
    • \$\begingroup\$Thank you. But why not consistent\$\endgroup\$CommentedJul 28, 2024 at 3:15

    Start asking to get answers

    Find the answer to your question by asking.

    Ask question

    Explore related questions

    See similar questions with these tags.