ibnhamza ibnhamza - 3 months ago 18
ASP.NET (C#) Question

How can I avoid over usage of ViewBag

I have this project I'm working on. In the index view I have to display a lot of data that are probably not related. Currently my index action in the controller is looking totally messed up as I can say I've done it in a very amateurish way. I've read a lot about usage of ViewBag and some even say one should avoid using it totally and use a ViewModel instead. I've been implementing ViewModel and I understand the usefulness and I appreciate it. However in this project I don't know how to fashion my request into one view model because I'm querying some models that are not related so I can't start using join statements. See code below

public ActionResult Index()
{

var db = new ApplicationDbContext();
var manager = new UserManager<ApplicationUser>(new UserStore<ApplicationUser>(db));
var CurrentUser = manager.FindById(User.Identity.GetUserId());
/*******counts of loans*******/
var PendingCounts = db.Transaction.Where(t => t.Status.Status == "Pending" && t.AgentId == CurrentUser.SalesAgent.AgentId).Count();
var DeclinedCounts = db.Transaction.Where(t => t.Status.Status == "Declined" && t.AgentId == CurrentUser.SalesAgent.AgentId).Count();
var ApprovedCounts = db.Transaction.Where(t => t.Status.Status == "Approved" && t.AgentId == CurrentUser.SalesAgent.AgentId).Count();
ViewBag.PendingCounts = PendingCounts;
ViewBag.DeclinedCounts = DeclinedCounts;
ViewBag.ApprovedCounts = ApprovedCounts;
/***Percentage of pending, approved and declined loans in counts******/
decimal PendingCount = (decimal)PendingCounts;
decimal DeclinedCount = (decimal)DeclinedCounts;
decimal ApprovedCount = (decimal)ApprovedCounts;
decimal sum = PendingCount + DeclinedCount + ApprovedCount;
decimal PendingPercent = (PendingCount / sum) * 100;
decimal DeclinedPercent = (DeclinedCount / sum) * 100;
decimal ApprovedPercent = (ApprovedCount / sum) * 100;
ViewBag.PendingPercent = PendingPercent;
ViewBag.DeclinedPercent = DeclinedPercent;
ViewBag.ApprovedPercent = ApprovedPercent;
/****Value of loans******/
//select SUM(AmountRequested) as totalPending from Transactions where StatusId = 1;
var ValuePending = db.Transaction.Where(t => t.AgentId == CurrentUser.SalesAgent.AgentId && t.Status.Status == "Pending").Select(t => (decimal?)t.AmountRequested).Sum() ?? 0;
var ValueApproved = db.Transaction.Where(t => t.AgentId == CurrentUser.SalesAgent.AgentId && t.Status.Status == "Approved").Select(t => (decimal?)t.AmountApproved).Sum() ?? 0;
var ValueDeclined = db.Transaction.Where(t => t.AgentId == CurrentUser.SalesAgent.AgentId && t.Status.Status == "Declined").Select(t => (decimal?)t.AmountRequested).Sum() ?? 0;
ViewBag.ValuePending = ValuePending;
ViewBag.ValueApproved = ValueApproved;
ViewBag.ValueDeclined = ValueDeclined;
var thisDate = DateTime.Now;
var starting = new DateTime(thisDate.Year, thisDate.Month, 1);
var ending = starting.AddMonths(1);
var SpecifiedTotal = (from t in db.Transaction
where t.AgentId == CurrentUser.SalesAgent.AgentId
&& t.Status.Status == "Approved"
&& t.DateApproved >= starting && t.DateApproved < ending
select (decimal?)t.AmountApproved).Sum() ?? 0;

var MonthlyTarget = db.SalesAgent.Where(t => t.AgentId == CurrentUser.SalesAgent.AgentId).Select(t => (decimal)t.MonthlyTarget).FirstOrDefault();
var today = DateTime.Now;
var year = DateTime.Now.Year;
var month = DateTime.Now.Month;
var day = DateTime.Now.Day;
var NoOfDays = DateTime.DaysInMonth(year, month);

var start = today;
var stop = new DateTime(year, month, NoOfDays);

int WorkingDaysLeft = GetNumberOfWorkingDays(start, stop);
decimal RunRate = Math.Round((MonthlyTarget - SpecifiedTotal) / WorkingDaysLeft, 2);
ViewBag.RunRate = RunRate.ToString("N0");
/***Get initials of current user*///
var firstname = db.SalesAgent.Where(s => s.AgentId == CurrentUser.SalesAgent.AgentId).Select(s => s.AgentFirstName.Substring(0, 1)).SingleOrDefault();
var lastname = db.SalesAgent.Where(s => s.AgentId == CurrentUser.SalesAgent.AgentId).Select(s => s.AgentLastName.Substring(0, 1)).SingleOrDefault();
//ViewBag.firstnameletter = firstnameletter;
//ViewBag.lastnameletter = lastnameletter;
return View();
}


as you can see I've stuffed a lot of data into the ViewBag because I need to show it in my view for example my view looks like this

@{
ViewBag.Title = "Home Page";
}

<div class="imagecircle">
@ViewBag.firstnameletter @ViewBag.lastnameletter
</div>

<div class="row searchForm">
<form action="/Home/Index" method="post">

<div class="col-lg-3">
<label>Start</label>
<input type="date" class="StartDate form-control" name="StartDate" />
</div>
<div class="col-lg-3">
<label>End</label>
<input type="date" class="EndDate form-control" name="EndDate" />
</div>
<input type="submit" value="Search" class="submit btn btn-default " />

</form>
</div>

<div class="row header">
<div class="col-lg-3 section">
<div class="row">
<div class="col-lg-8">
<h4><span class="countNumber">@ViewBag.PendingCounts</span></h4> Transaction(s) Pending
<h4> <span class="countNumber">@ViewBag.PendingPercent</span>%</h4> Transaction(s) Pending
<h4><span class="countNumber">@ViewBag.ValuePending</span></h4> in value so far
</div>
<div class="col-lg-4 countHeader">
<i class="fa fa-spinner fa-spin"></i>
<h5>Pending</h5>
</div>
</div>
</div>
<div class="col-lg-3 section">
<div class="row">
<div class="col-lg-8">

<h4><span class="countNumber">@ViewBag.ApprovedCounts</span></h4> Transaction(s) Approved
<h4><span class="countNumber">@ViewBag.ApprovedPercent</span>%</h4> Transaction(s) Approved
<h4><span class="countNumber">@ViewBag.ValueApproved</span></h4> in value so far

</div>
<div class="col-lg-4 countHeader">
<i class="fa fa-check-circle-o"></i>
<h5>Approved</h5>
</div>
</div>
</div>
<div class="col-lg-3 section">
<div class="row">
<div class="col-lg-8">
<h4><span class="countNumber">@ViewBag.DeclinedCounts</span></h4> Transaction(s) Declined
<h4><span class="countNumber">@ViewBag.DeclinedPercent</span>%</h4> Transaction(s) Declined
<h4><span class="countNumber">@ViewBag.ValueDeclined</span></h4> in value so far
</div>
<div class="col-lg-4 countHeader">
<i class="fa fa-remove"></i>
<h5>Declined</h5>
</div>
</div>
</div>
</div>



<div class="row">
<div class="col-lg-12 RunRate">
<p><sup><i class="fa fa-exclamation-circle"></i></sup>Run Rate = &#8358 @ViewBag.RunRate</p>
<p>This is the amount you should get disbursed daily in order to meet your target this month.</p>
</div>
</div>


Because I needed to perform lots of calculations on the results of my query I find it difficult to use a ViewModel hence I'm stuck with ViewBag. I know this is far from professional as it doesn't look clean and smooth. I think I will try to refactor those single query where I'm expecting a single result into methods, but what improvement do you think I need to do on the code above? Do help with code examples.

Answer

ViewModels store the presentation logic needed for a view.

Although I question why many of the calculations are not part of the models themselves I leave it as a refactoring exercise to remove the business logic from the viewmodel into the model.

Additionally I leave refactoring the view to use the viewmodel as an exercise of the reader.

Here is an example of a viewmodel for your stuff:

public class TransactionViewModel
{
    public TransactionViewModel(
        List<Transaction> transactions,
        SalesAgent salesAgent)
    {
        Transactions = transactions;
        SalesAgent = salesAgent;
    }

    public List<Transaction> Transactions { get; set; }

    public SalesAgent SalesAgent { get; set; }

    public decimal MonthlyTarget
    {
        get { return (decimal) SalesAgent.MonthlyTarget; }
    }

    public string SalesAgentInitials
    {
        get
        {
            return
                string.Format(
                    "{0}.{1}.",
                    SalesAgent.FirstName.First(),
                    SalesAgent.LastName.First());
        }
    }

    public IEnumerable<Transaction> ApprovedTransactions
    {
        get { return Transactions.Where(t => t.Status.Status == "Approved"); }
    }

    public IEnumerable<Transaction> PendingTransactions
    {
        get { return Transactions.Where(t => t.Status.Status == "Pending"); }
    }

    public IEnumerable<Transaction> DeclinedTransactions
    {
        get { return Transactions.Where(t => t.Status.Status == "Declined"); }
    }

    public int ApprovedCount
    {
        get { ApprovedTransactions.Count(); }
    }

    public int PendingCount
    {
        get { PendingTransactions.Count(); }
    }

    public int DeclinedCount
    {
        get { DeclinedTransactions.Count(); }
    }

    public int TotalCount
    {
        get { return Transactions.Count; }
    }

    public decimal ApprovedPercent
    {
        get { return (decimal) ApprovedCount / TotalCount }
    }

    public decimal PendingPercent
    {
        get { return (decimal) PendingCount / TotalCount }
    }

    public decimal DeclinedPercent
    {
        get { return (decimal) DeclinedCount / TotalCount }
    }

    public decimal ApprovedValue
    {
        get { return ApprovedTransactions.Select(t => (decimal?)t.AmountRequested).Sum() ?? 0; }
    }

    public decimal PendingValue
    {
        get { return PendingTransactions.Select(t => (decimal?)t.AmountRequested).Sum() ?? 0; }
    }

    public decimal DeclinedValue
    {
        get { return DeclinedTransactions.Select(t => (decimal?)t.AmountRequested).Sum() ?? 0; }
    }

    public DateTime StartMonth
    {
        get { return new DateTime(DateTime.Today.Year, DateTime.Today.Month, 1); }
    }

    public DateTime EndMonth
    {
        get { return StartMonth.AddMonths(1); }
    }

    public int DaysInMonth
    {
        get { return (EndMonth - StartMonth).Days; }
    }

    public int WorkingDaysLeftInMonth
    {
        get
        {
            Enumerable
                .Range(DateTime.Today.Day, DaysInMonth)
                .Select(d => new DateTime(DateTime.Today.Year, DateTime.Today.Month, d))
                .Count(date =>
                    date.DayOfWeek != DayOfWeek.Saturday &&
                    date.DayOfWeek != DayOfWeek.Sunday);
        }
    }

    public decimal SpecifiedTotal
    {
        get
        {
            return
                ApprovedTransactions
                    .Where(t => t.DateApproved >= StartMonth && t.DateApproved < EndMonth)
                    .Select(t => (decimal?) t.AmountApproved)
                    .Sum() ?? 0;
        }
    }

    public decimal RunRate
    {
        get { return Math.Round((MonthlyTarget - SpecifiedTotal) / WorkingDaysLeftInMonth, 2); }
    }

    public string RunRateFormat
    {
        get { return RunRate.ToString("N0"); }
    }
}

This simplifies the action:

public ActionResult Index()
{
    var db = new ApplicationDbContext();    
    var manager = new UserManager<ApplicationUser>(new UserStore<ApplicationUser>(db));
    var CurrentUser = manager.FindById(User.Identity.GetUserId());

    var loans =
        db.Transaction
          .Where(t =>
              t.AgentId == CurrentUser.SalesAgent.AgentId)
          .ToList();

    var salesAgent = 
        db.SalesAgent
          .FirstOrDefault(s => s.AgentId == CurrentUser.SalesAgent.AgentId);

    var viewModel = new TransactionViewModel(loans, salesAgent)

    return View(viewModel);
}