0

I wrote a code which return a list from 2 different Databases. The joint field between these tow dbcontext is accountid and email (both has a same value). Since there are 2 different databases, I can't use join in entity framework. So I used a nested using and for each blocks. Here is my code:

namespace AdminMvc.Components.BankDepositHistory
{
    public class BankDepositHistoryHelper
    {
        public static List<BankDepositHistoryItemDto> GetChangeRequestsList(int skip, int take, out int total, string name, string email, AvailableBankDepositStates state)
        {
            using (var myketAdsDB = new MyketAdsEntities())
            {
                using (var myketDB = new MyketReadOnlyDb())
                {
                    #region DefaultQuery
                    var bankDepositHistories = myketAdsDB.BankDepositHistories.AsQueryable();
                    #endregion
                    #region Filtering
                    if (!string.IsNullOrWhiteSpace(name))
                    {
                        var emails = myketDB.AppDevelopers
                            .Where(n => n.RealName.Contains(name))
                            .Select(e => e.Email).ToList();
                        // emails.Add(email);
                        if (emails.Count > 0)
                        {
                            bankDepositHistories = bankDepositHistories.Where(e => emails.Contains(e.AccountId));
                        }
                    }
                    if (!string.IsNullOrWhiteSpace(email))
                    {
                        bankDepositHistories = bankDepositHistories.Where(a => a.AccountId.Contains(email));
                    }
                    if (state != AvailableBankDepositStates.All)
                    {
                        bankDepositHistories = state == AvailableBankDepositStates.Success ?
                        bankDepositHistories.Where(x => x.State == AvailableBankDepositStates.Success.ToString()) :
                        bankDepositHistories.Where(x => x.State == AvailableBankDepositStates.Fail.ToString());
                    }
                    else
                    {
                        bankDepositHistories = bankDepositHistories.
                            Where(x => x.State != BankDepositState.Start.ToString());
                    }
                    #endregion
                    #region GetingTotalpages
                    total = bankDepositHistories.Count();
                    #endregion
                    #region Pagination
                    var pageResult = bankDepositHistories.OrderByDescending(ba => ba.CreationDate).Skip(skip).Take(take).ToList();
                    #endregion
                    #region FillingDomainObjects
                    var emailFilter = pageResult.Select(r => r.AccountId).ToList();
                    var developers = myketDB.AppDevelopers.Where(a => emailFilter.Contains(a.Email)).
                        Select(r => new { r.RealName, r.Email }).ToList();
                    var result = pageResult
                        .Select(b => new BankDepositHistoryItemDto()
                        {
                            Id = b.Id,
                            AccountId = b.AccountId,
                            Amount = b.Amount,
                            ClientIp = b.ClientIp,
                            State = (BankDepositState)Enum.Parse(typeof(BankDepositState), b.State, true),
                            ReturnUrl = b.ReturnUrl,
                            AdditionalData = b.AdditionalData,
                            Gateway = b.Gateway,
                            CreationDate = b.CreationDate,
                            PaymentRefNumber = b.PaymentRefNumber,
                            Uuid = b.Uuid,
                        }).ToList();
                    foreach (var bankDepositHistory in result)
                    {
                        foreach (var developer in developers)
                        {
                            if (bankDepositHistory.AccountId == developer.Email)
                            {
                                bankDepositHistory.RealName = developer.RealName;
                            }
                        }
                    }
                    return result;
                    #endregion
                }
            }
        }

I was wondering if it's possible to avoid using nested using and write a separated using for each database.

Ilyes
  • 14,640
  • 4
  • 29
  • 55
Ali Eshghi
  • 1,131
  • 1
  • 13
  • 30
  • Why do you want to have two separate `using` statements? What value does that give you? – Enigmativity May 01 '19 at 08:28
  • 1
    you can use 2(or more) `using` statements one below the other without using an opening brackets – styx May 01 '19 at 08:31
  • @Enigmativity because my boss want it :) . there is no problem for query . query return list of object. – Ali Eshghi May 01 '19 at 08:31
  • @AliEshghi - why does your boss want it? Surely he/she has explained the reason. – Enigmativity May 01 '19 at 08:34
  • @styx I know that. but I want two different. – Ali Eshghi May 01 '19 at 08:35
  • @AliEshghi two different what? – styx May 01 '19 at 08:36
  • @Enigmativity I even searched in the net for the reason but I don't know why he say that! he say this is bad practice and make object heavy ! – Ali Eshghi May 01 '19 at 08:38
  • @styx 2 different using not nested or below the other. – Ali Eshghi May 01 '19 at 08:39
  • 1
    @AliEshghi - It's probably not worth optimising unless you know that you're creating a memory issue. However your code is really quite convoluted. If I were your manager I would expect you to simplify the code significantly, and that's just from a maintenance POV. I'm trying to refactor your code, but it's hard, real hard. – Enigmativity May 01 '19 at 09:21
  • @AliEshghi - Can someone pass in both `name` and `email`? It seems to me that this would only be called by either `name` or `email`. – Enigmativity May 01 '19 at 09:37
  • @AliEshghi - It also seems to me that if someone passes in an invalid `name` then this just returns all the `bankDepositHistories` to that point. That's seems wrong. – Enigmativity May 01 '19 at 09:40
  • @Enigmativity no its work and its return nothing. thanks for your helping let me check cragin answer. – Ali Eshghi May 01 '19 at 09:43
  • @AliEshghi - Sorry, what works and returns nothing? Which of my comments are you replying to? – Enigmativity May 01 '19 at 09:46

2 Answers2

1

You can do what you're asking. The list of emails from the inner using affects the bankDepositHistories, which are from the outer using, but that outer query isn't executed until later. (Also, the original inner using doesn't depend on anything in the outer, so can be moved outside of it).

So, get the email list first, using myketDB:

List<Email> emails = new List<Email>();
using (var myketDB = new MyketReadOnlyDb())
{
    if (!string.IsNullOrWhiteSpace(name))
    {
        emails = myketDB.AppDevelopers
            .Where(n => n.RealName.Contains(name))
            .Select(e => e.Email).ToList();
    }
}

// original outer using is now after the above

Then do all the other logic by moving the outer using of myketAdsDB in your original code below the using above. This is now one after the other, not nested.

If what you are doing does not have to be transactional, accessing the contexts sequentially is preferred because you don't have to extend the lifetime of the outer context for no reason. Running an inner inside of an outer extends the life of the outer.

Kit
  • 20,354
  • 4
  • 60
  • 103
1

Your code is very convoluted. This is the best that I could do to separate and simplify:

public static List<BankDepositHistoryItemDto> GetChangeRequestsList(int skip, int take, out int total, string name, string email, AvailableBankDepositStates state)
{
    var statesFilter = new Dictionary<AvailableBankDepositStates, Func<IQueryable<BankDepositHistory>, IQueryable<BankDepositHistory>>>()
    {
        { AvailableBankDepositStates.All, bdh => bdh.Where(x => x.State != BankDepositState.Start.ToString()) },
        { AvailableBankDepositStates.Success, bdh => bdh.Where(x => x.State == AvailableBankDepositStates.Success.ToString()) },
        { AvailableBankDepositStates.Fail, bdh => bdh.Where(x => x.State == AvailableBankDepositStates.Fail.ToString()) },
    };

    List<string> emails = new List<string>();
    ILookup<string, string> developers = null;

    using (var myketDB = new MyketReadOnlyDb())
    {
        if (!string.IsNullOrWhiteSpace(name))
        {
            emails = myketDB.AppDevelopers.Where(n => n.RealName.Contains(name)).Select(e => e.Email).ToList();
        }
        developers = myketDB.AppDevelopers.ToLookup(x => x.Email, x => x.RealName);
    }

    using (var myketAdsDB = new MyketAdsEntities())
    {
        var bankDepositHistories = myketAdsDB.BankDepositHistories.AsQueryable();
        if (emails.Count() > 0)
        {
            bankDepositHistories = bankDepositHistories.Where(e => emails.Contains(e.AccountId));
        }
        if (!string.IsNullOrWhiteSpace(email))
        {
            bankDepositHistories = bankDepositHistories.Where(a => a.AccountId.Contains(email));
        }
        bankDepositHistories = statesFilter[state](bankDepositHistories);

        total = bankDepositHistories.Count();

        var result =
            bankDepositHistories
                .OrderByDescending(ba => ba.CreationDate)
                .Skip(skip)
                .Take(take)
                .ToList()
                .Select(b => new BankDepositHistoryItemDto()
                {
                    Id = b.Id,
                    AccountId = b.AccountId,
                    Amount = b.Amount,
                    ClientIp = b.ClientIp,
                    State = (BankDepositState)Enum.Parse(typeof(BankDepositState), b.State, true),
                    ReturnUrl = b.ReturnUrl,
                    AdditionalData = b.AdditionalData,
                    Gateway = b.Gateway,
                    CreationDate = b.CreationDate,
                    PaymentRefNumber = b.PaymentRefNumber,
                    Uuid = b.Uuid,
                    RealName = developers[b.AccountId].LastOrDefault(),
                }).ToList();

        return result;
    }
}

Here is the code I had to write to refactor safely:

public enum AvailableBankDepositStates
{
    All, Success, Fail
}

public enum BankDepositState
{
    Start
}

public class BankDepositHistoryItemDto
{
    public string AccountId;
    public BankDepositState State;
    public DateTime CreationDate;
    public string RealName;
}

public class MyketAdsEntities : IDisposable
{
    public IEnumerable<BankDepositHistory> BankDepositHistories;

    public void Dispose()
    {
        throw new NotImplementedException();
    }
}

public class MyketReadOnlyDb : IDisposable
{
    public IEnumerable<AppDeveloper> AppDevelopers;

    public void Dispose()
    {
        throw new NotImplementedException();
    }
}

public class BankDepositHistory
{
    public string AccountId;
    public string State;
    public DateTime CreationDate;
}

public class AppDeveloper
{
    public string RealName;
    public string Email;
}
Enigmativity
  • 113,464
  • 11
  • 89
  • 172