locked
Business logic refactoring help needed RRS feed

  • Question

  • User-1104215994 posted

    Hello guys,

    I am using asp.net web API and EF code first. I am querying the database based on values inside of the request object (initiate). If there are results, I am updating the database. Then I am querying this updated results in the database and save it to another table (initiateResponse).

    This is a very fat controller, it needs to be refactored. I read some articles and decided to use <g class="gr_ gr_21 gr-alert gr_gramm gr_inline_cards gr_run_anim Grammar only-ins doubleReplace replaceWithoutSep" id="21" data-gr-id="21">repository</g> pattern and unit of work for transactions. I need your help to refactor this controller portion. Any suggestions?

    Should I need TransactionScope or UOW handles it?

    #region Query 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 from 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

    I implemented an interface as follows:

    public interface IApiController<TDto, T>
        {
            #region GetMethods
            Task<ApiResult<TDto>> FindAsync(TDto item);
            #endregion
    
            #region PostMethods
            ApiResult<TDto> Add(TDto item);
            Task<ApiResult<TDto>> AddAsync(TDto item);
            ApiResult<TDto> Update(TDto item);
            Task<ApiResult<TDto>> UpdateAsync(TDto item);
            
            #endregion
        }

    Here is my <g class="gr_ gr_30 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="30" data-gr-id="30">reposirtory</g>:

    public class GenericRepository<T> : IGenericRepository<T> where T : class
        {
            #region Variables
            
            private DbContext _context;
            private DbSet<T> _dbset;
            #endregion
    
            #region Constructor
            
            public GenericRepository(DbContext context)
            {
                
                _context = context;
                _dbset = _context.Set<T>();
            }
            #endregion
    
            #region GetterMethods
            
            public IQueryable<T> GetAll()
            {
                return _dbset;
            }
    
           
            public T Find(Guid Id)
            {
                return _dbset.Find(Id);
            }
            #endregion
    
            #region SetterMethods
           
            public T Update(T entityToUpdate)
            {
                _dbset.Attach(entityToUpdate);
                _context.Entry(entityToUpdate).State = EntityState.Modified;
                return entityToUpdate;
            }
    
            
            public T Add(T entity)
            {
                _dbset.Add(entity);
                return entity;
            }
    
            
           
            #endregion
    
        }

    Here is my UOW:

    public class UnitofWork : IUnitofWork
        {
            #region Variables
            
            private readonly DbContext _context;
            private DbContextTransaction _transaction;
            private bool _disposed;
            #endregion
    
            #region Constructor
            
            public UnitofWork(DbContext context)
            {
                _context = context;
            }
            #endregion
    
            #region BusinessSection
            
            public IGenericRepository<T> GetRepository<T>() where T : class
            {
                return new GenericRepository<T>(_context);
            }
    
            
            public bool BeginNewTransaction()
            {
                try
                {
                    _transaction = _context.Database.BeginTransaction();
                    return true;
                }
                catch (Exception ex)
                {
                    return false;
                }
            }
    
            
            public bool RollBackTransaction()
            {
                try
                {
                    _transaction.Rollback();
                    _transaction = null;
                    return true;
                }
                catch (Exception ex)
                {
                    return false;
                }
            }
    
            
            public int SaveChanges()
            {
                var transaction = _transaction ?? _context.Database.BeginTransaction();
                using (transaction)
                {
                    try
                    {
                        
                        if (_context == null)
                        {
                            throw new ArgumentException("Context is null");
                        }
                       
                        int result = _context.SaveChanges();
    
                        
                        transaction.Commit();
                        return result;
                    }
                    catch (Exception ex)
                    {
                       
                        transaction.Rollback();
                        throw new Exception("Error on save changes ", ex);
                    }
                }
            }
            #endregion
    
            #region DisposingSection
            
            protected virtual void Dispose(bool disposing)
            {
                if (!this._disposed)
                {
                    if (disposing)
                    {
                        _context.Dispose();
                    }
                }
                this._disposed = true;
            }
    
            public void Dispose()
            {
                Dispose(true);
                GC.SuppressFinalize(this);
            }
            #endregion
    
        }

    Monday, June 10, 2019 10:25 AM

All replies

  • User1120430333 posted

    This is a very fat controller, it needs to be refactored. I read some articles and decided to use repository pattern and unit of work for transactions. I need your help to refactor this controller portion. Any suggestions?

    What are you saying is that all this code you are showing is in the controller? Is so, then yeah it's too fat IMO.

    You could be using an IoC and doing dependency inject of objects into the controller from a another folder in the project,  namespace seperation, or reference a classlib project another form of namespace speration, which are layered architectural styles, and using an Interface to decouple. 

    https://www.c-sharpcorner.com/blogs/understanding-interfaces-via-loose-coupling-and-tight-coupling

    In the below VB code, I am using the Unity IoC in the WebAPI project and DI the Data Access Object (DAO)  in the Data Access Layer (DAL)  into the controller. I can also make other folders or even use the Models folder and put a class in the folder using an Interface an DI the class/object into the controller and used its behavior.

    I can easily unit test the controller becuase I can mock out any class/object the controller is using due to the class/object is using an Interface.

    Imports System.Web.Http
    Imports DAL
    Imports Entities
    
    Namespace Controllers
    
        <CustomExceptionFilter>
        Public Class ProjectController
            Inherits ApiController
    
            Private ReadOnly _daoproject As IDaoProject
    
            public sub New (daoproject As IDaoProject)
                _daoproject = daoproject
            End sub
    
            <HttpGet>
            <ActionName("GetProjectById")>
            public Function GetProjectById(ByVal id As Int32) As DtoProject
                return _daoproject.GetProjectById(id)
            End Function
    
    
            <HttpGet>
            <ActionName("GetProjectsByUserId")>
            public Function GetProjectsByUserId(ByVal userid As String) As List(Of DtoProject)
                return _daoproject.GetProjectsByUserId(userid)
            End Function
    
            <HttpPost>
            <ActionName("CreateProject")>
            public sub CreateProject(ByVal dto As DtoProject)
                Call _daoproject.CreateProject(dto)
            End sub
            
            <HttpPost>
            <ActionName("UpdateProject")>
            public sub UpdateProject(ByVal dto As DtoProject)
                Call _daoproject.UpdateProject(dto)
            End sub
    
            <HttpPost>
            <ActionName("DeleteProject")>
            public sub  DeleteProject(ByVal dto As DtoId)
                Call _daoproject.DeleteProject(dto.Id)
            End sub
            
        End Class
    End Namespace

    Monday, June 10, 2019 12:42 PM
  • User-1104215994 posted

    Yes, the sample code is in an action method of a controller. I am trying to decouple things <g class="gr_ gr_60 gr-alert gr_spell gr_inline_cards gr_run_anim ContextualSpelling ins-del multiReplace" id="60" data-gr-id="60">thats</g> why I posted generic repo and unit of work samples. I couldn't figure out if I should use transaction scope or unit of work handles the transaction in my sample.

    Monday, June 10, 2019 12:59 PM
  • User1120430333 posted

    Yes, the sample code is in an action method of a controller. I am trying to decouple things thats why I posted generic repo and unit of work samples. I couldn't figure out if I should use transaction scope or unit of work handles the transaction in my sample.

    The UoW and the Repository at the very least should be in its own classlib project,  and the WebAPI project references the Repository classlib project. Of course, you would DI  the objects into the controller from an IoC container that has been setup in the WebAPI project. I don't use UoW or the generic Repository object, but I would think that a transaction scope would be initiated in the UoW and completed.

    Monday, June 10, 2019 1:21 PM