LINQ refactoring query

0

Na głównej stronie projektu chcę wyświetlić podsumowanie użytkownikowi związane z jego zadaniami. Mam taki kod, spełnia on swoje zadanie ale według mnie nie wygląda on dobrze. Jak można go poprawić? Będę wdzięczny za jakieś wzorce, techniki itp. Wydaje mi się, że coś w Where i Select trzeba zmienić bo za dużo jest duplikacji. Nie wiem czy ToList() dobrze użyłem, teraz będą 4 zapytania do bazy danych, można to jakoś zoptymalizować?

var summaryViewModel = new SummaryViewModel
{
	TodayTasks = _context.Tasks
		.Where(t => t.DueDate.Date == DateTime.Today.Date &&
					t.IsCompleted == false &&
					t.UserId == userId)
		.OrderByDescending(t => t.Priority)
		.Take(5)
		.Select(t => new TaskViewModel
		{
			Id = t.Id,
			Name = t.Name,
			DueDate = t.DueDate.ToLongDateString(),
			Priority = t.Priority.ToString(),
			Category = t.Category.Name
		})
		.ToList(),

	TomorrowTasks = _context.Tasks
		.Where(t => t.DueDate.Date == DateTime.Today.AddDays(1).Date &&
					t.IsCompleted == false &&
					t.UserId == userId)
		.OrderByDescending(t => t.Priority)
		.Take(5)
		.Select(t => new TaskViewModel
		{
			Id = t.Id,
			Name = t.Name,
			DueDate = t.DueDate.ToLongDateString(),
			Priority = t.Priority.ToString(),
			Category = t.Category.Name
		})
		.ToList(),

	LaterTasks = _context.Tasks
		.Where(t => t.DueDate.Date > DateTime.Today.AddDays(1).Date &&
					t.IsCompleted == false &&
					t.UserId == userId)
		.OrderBy(t => t.DueDate.Date)
		.ThenByDescending(t => t.Priority)
		.Take(5)
		.Select(t => new TaskViewModel
		{
			Id = t.Id,
			Name = t.Name,
			DueDate = t.DueDate.ToLongDateString(),
			Priority = t.Priority.ToString(),
			Category = t.Category.Name
		})
		.ToList(),

	RecentlyCompletedTasks = _context.Tasks
		.Where(t => t.IsCompleted &&
					t.UserId == userId)
		.OrderBy(t => t.CompletedAt.Value)
		.Take(5)
		.Select(t => new TaskViewModel
		{
			Id = t.Id,
			Name = t.Name,
			DueDate = t.DueDate.ToLongDateString(),
			Priority = t.Priority.ToString(),
			Category = t.Category.Name
		})
		.ToList()
};
0

Generalnie tak:

Rozbiłbym to na metody,

Wszystkie te selecty bym wrzucił do jednej metody

A całą resztę dałoby się bardzo ładnie skrócić gdyby nie te Ordery, bo warunki w Where spokojnie możesz dynamicznie podawać.

Jak wymyślisz jak podawać te Ordery jako parametry bez zabawy z czymś, co pogorszy przejrzystość tego kodu (np. refleksję lub dodatkowa metoda) to mamy to :P

0

A ja bym raz pobrał wszystkie Taski w jednym zapytaniu (warunek z userem oraz datą) a potem filtrował w pamięci już.

1

Czy pobierzesz sobie od strzala wszystkie Taski powyzej dzisiejszego czy dasz metody do budowy Iqueryable to juz kwestia dyskusyjna czy warto czy nie, ale generowanie modelu to sobie opakujw metode a unikniesz powtorzen i lamania zasady DRY cos takiugo np:

public IList<Task> GetTaskViewModel(IQueryable<Task>tasks, int noOfRecords = 5)
    {
        return tasks
        .Take(noOfRecords)
        .Select(t => new TaskViewModel
        {
            Id = t.Id,
            Name = t.Name,
            DueDate = t.DueDate.ToLongDateString(), 
            Priority = t.Priority.ToString(),
            Category = t.Category.Name
        })
        .ToList()
    }

I moje pytanie po co konwertujesz na stringi?? trzymnaj w modelu oryginalny typ a konwertuj na sam koniec ew pozwol na automatyczny casting.

0

Przeglądam kod i znalazłem w innym miejscu takie coś i znowu mam powtórzenie warunków @WeiXiao czyli tu proponujesz coś a la specification pattern i zwracać expression

return View("TasksViewComponent", new NumberOfTasksViewModel
{
	Today = _context.Tasks.Count(t => t.DueDate.Date == DateTime.Today.Date &&
									  t.IsCompleted == false &&
									  t.UserId == userId),

	Tomorrow = _context.Tasks.Count(t => t.DueDate.Date == DateTime.Today.AddDays(1).Date &&
										 t.IsCompleted == false &&
										 t.UserId == userId),

	Later = _context.Tasks.Count(t => t.DueDate.Date > DateTime.Today.AddDays(1).Date && 
	                                  t.IsCompleted == false &&
	                                  t.UserId == userId),

	NotDone = _context.Tasks.Count(t => t.DueDate.Date < DateTime.Today.Date && 
	                                    t.IsCompleted == false &&
	                                    t.UserId == userId),

	History = _context.Tasks.Count(t => t.IsCompleted &&
	                                    t.UserId == userId)
});
1

@Aenyatia:

Chociaż teraz to aż tak wiele nie skróci :P

public List<TaskViewModel> GetSpecificTasksCount(Expression<Func<Task, bool>> func)
{
	return _context.Tasks.Where(func).Count();
}

Gdyby jeszcze trzeba było tego Selecta walnąć do zmiany, to byłoby fajniej :P

public static List<TaskViewModel> Simplify(List<Task> tasks)
{
	return tasks.Select(t => new TaskViewModel
	{
		Id = t.Id,
		Name = t.Name,
		DueDate = t.DueDate.ToLongDateString(),
		Priority = t.Priority.ToString(),
		Category = t.Category.Name
	}).ToList();
}
1

ojej, tu jest troche mieszanie warstw. Stworz sobie jakis serwis ktory bedzie zaciagal z repozytorium te dane. Bo na chwile obecna w kontrolerze trzymasz referencje do kontekstu bazodanowego. A co jesli na pewnej stronie zaraz bedziesz potrzebowal liczbe samych taskow ktore sa NotDone. odwolasz sie do tej samej metody kontrolera ktora pobiera wszytsko?? czy bedziesz duplikowal kod piszac jesczze raz w kontrolerze zaciaganie taskow do modelu?? Wprowadz wiecej wartsw. przynajmniej serwis dla samych metod do wyciagania danych z bazy i jakies repo do kontaktu z baza bo za chwile bedziesz mial kilkanascie miejsc gdzie kod bedzie podobny bo wszystko bedzie upcahne w kontrolerach.

1 użytkowników online, w tym zalogowanych: 0, gości: 1