ASP.NET MVC5 czy prawidłowo zamimeplentowałem opcje filtrowania wyników z bazy?

0

Na stronie posiadam formularz w którym ustalam pola do filtrowania danych a następne tabelę, która te dane wyświetla. Po wysłaniu formularza kontroler dostaje obiekt SearchCriteria, który jest wypełniony ustawieniami, które wprowadził użytkownik.

[HttpPost]
public ActionResult Index(SearchCriteria search, int page = 1, int resultsPerPage = 100)

Teraz chciałbym pobrać te dane z bazy danych na podstawie kryteriów wyszukania. Stworzyłem więc klasę, która jest modelem opcji filtrowania

public class SearchCriteria 
{
    public DateTime? DateFrom { get; set; }
    public DateTime? DateTo { get; set; }
    public int? AmountFrom { get; set; }
    public int? AmountTo { get; set; }
    public string BankName { get; set; }
}

A następnie klasę, która podpowiada za pobieranie danych z bazy i ich filtrowanie:

public class HistorySearchBusinessLogic
{
    private DbBanki _context;

    public HistorySearchBusinessLogic()
    {
        _context = new DbBanki("db");
    }

    public IQueryable<History> GetHistories(SearchCriteria searchModel)
    {
        var result = _context.BankHistory.AsQueryable();
		
        if (searchModel != null)
        {
            if (searchModel.DateFrom.HasValue)
                result = result.Where(x => x.Received >= searchModel.DateFrom);
            if (searchModel.DateTo.HasValue)
                result = result.Where(x => x.Received <= searchModel.DateTo);		
			if (searchModel.AmountFrom.HasValue)
                    result = result.Where(x => x.Amount >= searchModel.AmountFrom);
            if (searchModel.AmountTo.HasValue)
                    result = result.Where(x => x.Amount <= searchModel.AmountTo);
            if (!string.IsNullOrEmpty(searchModel.BankName))
            {
                var bank = _context.BankAccounts.Where(x => x.Name == searchModel.BankName).FirstOrDefault();
                if (bank != null)
                    result = result.Where(x => x.Bankaccount.Id_BankShort == bank.Id);
            }
			
			
        }

        return result;
    }
}

Tak więc wygląda uzupełniony kontroler:

[HttpPost]
public ActionResult Index(SearchCriteria searchCriteria, int page = 1, int resultsPerPage = 100)
{
	var logic = new HistorySearchBusinessLogic();
	
	var results = logic.GetHistories(searchCriteria)
		.OrderBy(x => x.Id)
		.Skip((page-1) * resultsPerPage)
		.Take(resultsPerPage)
		.Select(x => new HistoryResultModel
		{
			Received = x.Received,
			Amount = x.Amount,
			BankName = x.BankAccount.Name
		})
		.ToList();
		
		return View(results);
}

Wyżej pisałem o HistoryResultModel. Jest to model, który odpowiada danym, które chce wyświetlić na mojej stronie:

public class HistoryResultModel
{
    public DateTime Received { get; set; }
    public int Amount { get; set; }
    public string BankName { get; set; }
}

I jak na razie wszystkie działa. Mam jednak kilka pytań:

  1. Najważniejsze - czy coś powinienem zmienić?

  2. W bazie Amount jest przechowywane jako int (czyli cena w groszach). Ja w tabeli chciałbym wyświetlić to w formacie złotówek z przecinkiem. Gdzie powinienem dokonać konwersji? W widoku?

static class MoneyFormatter {
    public static string PenniesToString(int pennies)
        => Convert.ToString(pennies / 100.0m);
}
<td>@MoneyFormatter.PenniesToString(Model.Amount)<td>

Czy może utworzyć ViewModel?

public class HistoryResultViewModel 
{
	public HistoryResultModel HistoryResultModel {get; set;}
	public string HistoryResultModelAmountFormatted {get; set;}
}

Albo po prostu dodać właściwość:

public class HistoryResultModel
{
    public DateTime Received { get; set; }
    public int Amount { get; set; }
	public string AmountFormatted { get { return Amount / 100m; } }
    public string BankName { get; set; }
}

A może coś innego?

  1. Co gdybym chciał zrobić identyczne filtrowanie, ale pobierającej dane z innej tabeli? Taki przypadek wystąpiłby, gdybym miał 2 tabele: archiwum i bieżące.
    Powinienem do metody GetHistories przkeazywać kontekst z którego ma korzystać? A może powinienem stworzyć klasę generyczną? A może wzorzec projektowy dekorator?

  2. Co gdybym chciał mieć przycisk, który eksportuje mi dane z tabelki? Powinienem zapisać przy każdym wyciąganiu danych zapisywać sobie je w sesji, by potem mógł z niej wyciągać przy eksportowaniu? Tak właściwie, to zaimplementowałem eksportowanie. Ma działać tak:
    Jeśli użytkownik nie zaznaczył w tabelce żadnego rekordu - eksportuj wszystkie
    Jeśli użytkownik zaznaczył jakiś rekordy - eksport zaznaczone
    Jeśli w tabeli nie ma rekordów - nie eskportuj
    Zaznaczanie rekordów odbywa się przez zaznaczenie CheckBox, który jest w ostatniej kolumnie

Obecnie działa to tak:

string guid = StringGenerator.GetRandomString(10);
ViewBag.dataGuid = guid;
Session[guid] = results;
Session[guid + "amount"] = elements;

return View("Index", search);

O co tu chodzi? Najpierw generuję sobie losowy ciąg znaków, który będzie identyfikował moje dane w sesji. W Session[guid + "amount"] zapisuję informacje o ilości danych, które zwróciło mi wyszukiwanie. Ale chwila, co to jest to elements? A no bo ja wyciągam z bazy danych zawsze resultsPerPage danych. Niezależnie od tego ile jest wyników, to zawsze wyciągnę np. 100 sztuk. Ale ja chcę mieć paginację! W takim razie potrzebuję 2 metody - jedna, która zwróci mi informację o tym ile jest wyników spełniających kryteria wyszukania oraz drugą, która zwróci mi np. pierwsze resultsPerPage wyników, które owe wyszukania spełniają.

Mam więc informację o tym ile łącznie jest wyników, więc mogę stworzyć paginację! W sesji zapisuję tą informację, bo gdy w tabeli nie ma zaznaczonego żadnego rekordu, to nie wiem, czy użytkownik chce wyeksportować wszystkie, czy nie ma żadnych do wyeksportowania.

Ok, w gruncie rzeczy sprowadza się do tego, że w sesji zapisuję wyniki wyszukiwania aktualnie wyświetlanej strony. Jeśli więc użytkownik chce wyeksportować rekordy z aktualnej strony, to pobieram je z sesji. jeśli natomiast chce wyeksportować wszystkie rekordy jakie spełniają kryteria wyszukania to niestety muszę ponownie je wyszukać.

Z widoku do kontrolera obsługującego eksportowanie przesyłam te rzeczy:

  • id zaznaczonych checkboxów (czyli id rekordu w bazie danych)
  • guid danych w sesji
  • kryteria wyszukania

I wszystko działa jak powinno - jeśli użytkownik eksportuje z aktualnie wyświetlanej strony wyniki dostaje od razu (bo są w sesji). Jeśli chce wyniki z wszystkich stron paginacji, to musi poczekać na zassanie ich z bazy.

Co sądzicie o takim rozwiązaniu? Troche jest karkołomne. Może po prostu powinienem za każdym razem przekazywać tylko informacje o zaznaczonych id rekordów oraz model kryteriów wyszukiwania i pobierać dane z bazy na nowo?

1
west napisał(a):
  1. Najważniejsze - czy coś powinienem zmienić?

Usunąć z kontrolera operowanie na źródle danych, IQueryable także nie powinno do kontrolera wyciekać. Paginacja i mapowanie na viewmodele to nie jest zadanie kontrolerów tylko warstwy niższej.

  1. W bazie Amount jest przechowywane jako int (czyli cena w groszach). Ja w tabeli chciałbym wyświetlić to w formacie złotówek z przecinkiem. Gdzie powinienem dokonać konwersji?

Do tego służy ViewModel.

  1. Co gdybym chciał zrobić identyczne filtrowanie, ale pobierającej dane z innej tabeli? Taki przypadek wystąpiłby, gdybym miał 2 tabele: archiwum i bieżące.
    Powinienem do metody GetHistories przkeazywać kontekst z którego ma korzystać? A może powinienem stworzyć klasę generyczną? A może wzorzec projektowy dekorator?

Ja bym zrobił dwie ładnie nazwane metody w publicznym API klasy, a że wiekszość kodu będzie identyczna, to pod spodem wywołają prywatną, generyczną.

  1. Co sądzicie o takim rozwiązaniu? Troche jest karkołomne. Może po prostu powinienem za każdym razem przekazywać tylko informacje o zaznaczonych id rekordów oraz model kryteriów wyszukiwania i pobierać dane z bazy na nowo?

Poza tym, że to jest dość skomplikowane rozwiązanie do prostego problemu, to przede wszystkim dopuszczasz sytuację, w której ktoś wyedytuje/usunie niektóre rekordy, a ktoś inny będzie mógł je wyeksportować, bo będzie je miał akurat wczytane w swoim widoku. I jaki w tym sens?

1

Przy powiązaniu relacji many to many, możesz mieć problem z dodaniem paginacji z tym filtrowaniem. Powinieneś całe filtrowanie i pobieranie zaimplementować w infrastrukturze a interfejs do tego wystawić w warstwie aplikacji, chyba że wolisz nazywać to zachowanie domeną :P jak to nie którzy mają do tego skłonność. Dalej powinieneś mieć oddzielny serwis dla paginacji oraz serwis odpowiadający za zachowanie aplikacji (Lub tej domeny.... ), do którego wstrzykniesz implementacje interfejsu serwisu, który pobiera i filtruje dane, ten serwis będzie również korzystał z serwisu paginacji. Równie dobrze możesz serwis paginacji nazwać internalUtil zamiast serwis, bo jest tworzony wyłącznie w kontekście serwisu aplikacji.

Jeśli chcesz zrobić to generycznie to da się tylko, że musiałbyś się posłużyć refleksją. Jednak taki styl rozwiązywania generycznych problemów (w sposób generyczny) jest poza zasięgiem większości ludzi. Jak to mój kolega powyżej kiedyś powiedział. ;)

0
somekind napisał(a):

Usunąć z kontrolera operowanie na źródle danych, IQueryable także nie powinno do kontrolera wyciekać. Paginacja i mapowanie na viewmodele to nie jest zadanie kontrolerów tylko warstwy niższej.

Dokumentacja pokazuje przykład gdzie operacje na źródle są w kontrolerze:
title
Jak nie tu, to gdzie powinno to być?

somekind napisał(a):

Paginacja i mapowanie na viewmodele to nie jest zadanie kontrolerów tylko warstwy niższej.
No to w kontrolerze mam pobrać z bazy wszystkie wyniki, a wyciągnąć tyle ile potrzebuje dopiero w View Modelu?

somekind napisał(a):

Poza tym, że to jest dość skomplikowane rozwiązanie do prostego problemu, to przede wszystkim dopuszczasz sytuację, w której ktoś wyedytuje/usunie niektóre rekordy, a ktoś inny będzie mógł je wyeksportować, bo będzie je miał akurat wczytane w swoim widoku. I jaki w tym sens?

Czyli jak to powinienem rozwiązać? Przesłać ponownie obiekt z ustawieniami wyszukania, pobrać elementy z bazy na nowo i dopiero wyeksportować?

2
west napisał(a):

Dokumentacja pokazuje przykład gdzie operacje na źródle są w kontrolerze:

Dokumentacja to nie jest źródło wiedzy o architekturze. Ona pokazuje jak korzystać z frameworka, nie jak pisać aplikacje.

Jak nie tu, to gdzie powinno to być?

Ja bym to umieścił w jakimś serwisie aplikacyjnym - tak zazwyczaj nazywam klasy wywoływane z kontrolera i zajmujące się operacjami na danych, w przypadku, gdy nie ma jakiejś większej logiki do wykonania.
W Twoim kodzie taką rolę spełnia HistorySearchBusinessLogic - tylko nazwa trochę dziwna, bo żadnej logiki biznesowej tam nie ma.

No to w kontrolerze mam pobrać z bazy wszystkie wyniki, a wyciągnąć tyle ile potrzebuje dopiero w View Modelu?

Nie. Żadnej bazy w kontrolerach. Operacje na niej mają się odbywać w niższych warstwach.

Czyli jak to powinienem rozwiązać? Przesłać ponownie obiekt z ustawieniami wyszukania, pobrać elementy z bazy na nowo i dopiero wyeksportować?

Jeśli chcesz eksportować prawdziwe dane, to tak.

0

Mam jeszcze jedno pytanie:
Mój formularz w kilku miejscach będzie wypełniany danymi z bazy danych (kontrolka dropdown). Powiedzmy, że jest to lista najpopularniejszych imion w mojej bazie danych i zaznaczając odpowiedni element w tym dropdown mogę filtrować wyniki z formularza na podstawie tego imienia.

Mogę więc zrobić to na 2 sposoby (pewnie są też inne):

  1. Informacje o najpopularniejszych imionach przekażę w ViewModel a następnie wyświetlę
@Html.DropDownListFor(model => model.PopularNames))
  1. W widoku użyć Html.Action, który pobierze mi listę najpopularniejszych imion i zwróci PartialView. Które rozwiązanie jest lepsze?

Jeśli pierwsze rozwiązanie jest lepsze, to mój ViewModel powinien zawierać List<SelectListItem, czy List<PopularNames>, czy może List<string>? W pierwszym przypadku nie musiałbym w widoku zamieniać kolekcji na SelectListItem, ale za to jest teraz mniej generyczna. Wydaje mi się, że miałoby to wyglądać tak:

[HttpGet]
public ActionResult Index()
{
    var vm = new HistoryViewModel()
    {
        OperationDate = DateTime.Now,
        SaveDate = DateTime.Now,
        PopularNames = _context.BankShortTypes.Select(x => new SelectListItem { Text = x.Name }).ToList();
		PopularNamesAsString = string.Join(",", _context.BankShortTypes.Select(x => x.Name ).ToList());
    };

    return View(vm);
}

public class HistoryViewModel
{
    public DateTime OperationDate { get; set; }
    public DateTime SaveDate { get; set; }

    public List<SelectListItem> PopularNamesList { get; set; }
	public string PopularNames {get; set;}
}

Pomijam już pobieranie danych w kontrolerze - na wszystkich poradnikach jakie widziałem pokazywano taki sposób. Może znajdę gdzieś sposób o którym piszesz somekind.

0
west napisał(a):

Jeśli pierwsze rozwiązanie jest lepsze, to mój ViewModel powinien zawierać List<SelectListItem, czy List<PopularNames>, czy może List<string>? W pierwszym przypadku nie musiałbym w widoku zamieniać kolekcji na SelectListItem, ale za to jest teraz mniej generyczna. Wydaje mi się, że miałoby to wyglądać tak:

Jeśli wybierzesz string, to będziesz miał dużo zmian, gdy okaże się, że potrzebujesz także id (a to najbardziej typowe zastosowanie dropdowna).
Jeśli wybierzesz SelectListItem, to gdy przeniesiesz tworzenie viewmodeli do niższej warstwy, to w testach jednostkowych będzie Ci się jakiś System.Web.Mvc pałętał. Nie wiem, czy to dobrze.

Pomijam już pobieranie danych w kontrolerze - na wszystkich poradnikach jakie widziałem pokazywano taki sposób. Może znajdę gdzieś sposób o którym piszesz somekind.

http://commitandrun.pl/2016/05/30/Brutalne_prawdy_o_MVC/

0

Możesz użyć DropDownListFor Jak pamiętam, możesz mu podać własny typ.

0
somekind napisał(a):

http://commitandrun.pl/2016/05/30/Brutalne_prawdy_o_MVC/

W takim razie gdzie mogę się nauczyć dobrych praktyk przy pisaniu aplikacji ASP.NET MVC5? Większość tutoriali pokazuje zdaje się złe praktyki w tym dokumentacja msdn. Skąd mam wiedzieć, czy pisana przeze mnie aplikacja jest napisana dobrze, skoro nie wiem, jak dobrze napisana aplikacja wygląda? Czy są jakieś przykłady aplikacji ASP.NET MVC5 które są dobrze napisane? Czy jest jakaś książka, które odpowie mi na moje pytanie?

0

Dużo czytać, dużo analizować, myśleć.
Jeszcze raz myśleć i analizować, zamiast implementować jakieś idiotyczne hasła typu: Każda klasa musi mieć jedną publiczną metodę. Każdy ekstra kod napisany dla testu jest złym kodem. CQRS'a potrzebuje mediatora, z mediatorem twój kontroler będzie miał mniej kodu. Do dependency injection jest potrzebny interfejs. Mój serwis potrzebuje repository, żebym mógł mockować bazę danych. Mógłbym tak wymieniać bez końca. Przemysł prelekcji i szkoleniowy robi swoje ;).

0

Na początek musisz zrozumieć co to jest SOC, Dependecy Inversion (równie w konteście eventów), Inversion Of Control (Composition Root, zależności pomiędzy pakietami). Bez tego będzie ci trudno zrozumieć jakąkolwiek architecturę.

0

@somekind @Enter Name

Moglibyście rzucić okiem na ten szablon projektu, który tytułuje się: ASP.NET MVC Solution Architecture – Best Practices

Kontroler wygląda tak:
title
Wydaje mi się, że nie jest to poprawnie zaimplementowane. Z bazy pobieramy informacje o kategorii, a następnie mapujemy to na viewModel by przekazać to do widoku. Wychodzi więc na to, że z bazy pobraliśmy nadmiarową ilość danych. CategoryViewModel wygląda tak:
title
natomiast z bazy pobraliśmy:
title

Pobraliśmy więc informacje o dacie utworzenia i zmodyfikowania kategorii, chociaż wcale nie były nam one potrzebne.

Czy nie lepiej byłoby, gdybyśmy za bazy pobierali tylko tyle informacji ile potrzebujemy przekazać do widoku? Czy zawsze powinienem dążyć do tego, by pobierać tylko tyle danych ile potrzebuję, czy nie trzeba aż tak bardzo przywiązywać do tego uwagi?

Czy takie rozwiązanie jest ok?

kontroler

var categories = categoryService.GetAll<CategoryViewModel>();

serwis kategorii

public IEnumerable<T> GetAll<T>()
{
    return categorysRepository.GetAll<T>();
}

repozytorium kategorii

IEnumerable<T> IRepository<Category>.GetAll<T>()
{
    return base.GetAll<T>();
}

klasa bazowa repozytorium

public virtual IEnumerable<R> GetAll<R>()
{
    return dbSet.ProjectTo<R>().ToList();
}

Użyty został automapper z Queryable Extensions. Finalnie w kontrolerze otrzymujemy tylko to, co potrzebowaliśmy:
title

0

Tradycyjnie repozytorium nazwane jest coś, co nie ma z repozytorium nic wspólnego.
Jaki jest zysk z aż dwóch warstw wrapperów na ORMa? Bo ja nie widzę żadnego.

0
[HttpPost]
        public ActionResult Create(GadgetFormViewModel newGadget)
        {
            if (newGadget != null && newGadget.File != null)
            {
                var gadget = Mapper.Map<GadgetFormViewModel, Gadget>(newGadget);
                gadgetService.CreateGadget(gadget); 

                string gadgetPicture = System.IO.Path.GetFileName(newGadget.File.FileName);<--- ????....
                string path = System.IO.Path.Combine(Server.MapPath("~/images/"), gadgetPicture);<--- ????....
                newGadget.File.SaveAs(path); <--- ????....

                gadgetService.SaveGadget(); <--- ????....
            }

            var category = categoryService.GetCategory(newGadget.GadgetCategory);
            return RedirectToAction("Index", new { category = category.Name });
        }

public class GadgetFormViewModel
    {
        public HttpPostedFileBase File { get; set; } <--- ????....
        public string GadgetTitle { get; set; }

Best Practice, powiadasz - ciekawe :|

0

@somekind @._.
A czy któryś z tych projektów jest napisany poprawnie? Pierwszy jest w 2 wersjach: angular / html + css + jquery
https://github.com/aspnetboilerplate/module-zero-template
https://github.com/NonFactors/MVC5.Template

Są w nich jakieś rażące błędy?

0
west napisał(a):

@somekind @._.
A czy któryś z tych projektów jest napisany poprawnie? Pierwszy jest w 2 wersjach: angular / html + css + jquery
https://github.com/aspnetboilerplate/module-zero-template
https://github.com/NonFactors/MVC5.Template

Są w nich jakieś rażące błędy?

https://github.com/NonFactors/MVC5.Template

UnitOfWork i Testy.

Przy drugim to nie wiem, VS mi się zawiesiło i dałem sobie spokój.

1

Hmm nie rozumiem trochę Twojej odpowiedzi. Wysłałeś link właśnie do drugiego projektu. Było to zamierzone

No właśnie jest na odwrót z tymi linkami. :|

UnitOfWork, nie powinien zawierać czegoś takiego jak "Get". :|

Jeśli chodzi o testy, to nic nie powiem, bo byłoby to w złym tonie. :|

Poza tym, ludzie raczej nie chętnie komentują czyiś kod publicznie, gdy właściciel o to nie pyta.

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