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.

Improve this question
13
  • Why do you want to have two separate using statements? What value does that give you? Commented May 1, 2019 at 8:28
  • 1
    you can use 2(or more) using statements one below the other without using an opening brackets Commented May 1, 2019 at 8:31
  • @Enigmativity because my boss want it :) . there is no problem for query . query return list of object. Commented May 1, 2019 at 8:31
  • @AliEshghi - why does your boss want it? Surely he/she has explained the reason. Commented May 1, 2019 at 8:34
  • 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. Commented May 1, 2019 at 9:21

2 Answers 2

Reset to default
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; }
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

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.

Improve this answer

1 Comment

I think you right let me try it then I will mark it as answer.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.