I miei controller ASP.NET MVC sembrano in disordine

0

Sto sviluppando un programma con asp.net MVC e mi sento come se stessi sbagliando da queste parti:

  1. Sto popolando tutto il processo nel controller, perché non so come inserirlo nel modello (come chiamarlo). Ho letto alcuni riferimenti e la gente dice che dobbiamo inserire il processo nel modello (?)

  2. Molte delle mie azioni hanno lo stesso processo (ripetitivo). Ad esempio, molte viste devono caricare i dati dalla stessa tabella SQL, quindi ripeto il processo per chiamare i dati in molte viste.

Qui fa parte del mio controller, puoi dirmi se sto facendo bene o sbagliato ecc.

public class EmployeeController : Controller
{
    private UserContext db = new UserContext();

    public async Task<ActionResult> Index()
    {

        string strquery = "Select employee.EmpId,EmpName,OrgName,EmpJobLvl,JobLvlName,EmpJobTtl,JobTtlName,employee.EmpType,"
                            + "EmpDateStart,EmpEmail,EmpHandphone,EmpJoinDate,EmpSignDate,EmpNPWP,EmpResAddr,city1.CityName AS CityName1,EmpResZipCode, "
                            + "EmpResPhone,EmpResStatus,EmpOriAddr,city2.CityName AS CityName2,EmpOriZipCode,EmpOriPhone,EmpOriStatus,EmpMaritalSt,EmpSex"
                            + " FROM employee "
                            + " join Organization (nolock) on employee.emporg = organization.orgcode "
                            + " join jobtitle (nolock) on employee.empjobttl = jobtitle.jobttlcode "
                            + " join JobLevel (nolock) on employee.EmpJobLvl = JobLevel.JobLvlCode"
                            + " join EmpType (nolock) on employee.EmpType = EmpType.EmpType"
                            + " INNER JOIN City city1 ON employee.EmpResCity = city1.CityCode"
                            + " INNER JOIN City city2 ON employee.EmpOriCity = city2.CityCode"
                            + " WHERE employee.EmpId = {0}";
        string rtsquery = "Select employee.EmpID,EmpFamRelation,EmpFamName,EmpFamDateBrith,CityName"
                            + ",EmpFamSex,EmpFamAlive,MaritalSt,EmpFamMaritalDate,EmploymentSt,EmpFamOccupation "
                            + ",EmpFamComp,EmpCompAddr,EmpFamEdu,EmpFamEduIns FROM employee  "
                            + "join EmpFamilys on employee.EmpId = EmpFamilys.EmpId "
                            + "join City on City.CityCode = EmpFamilys.EmpFamCityBirth "
                            + "WHERE employee.EmpId = {0}";
        string srtquery = "Select employee.EmpID,EmpEduStatus,EmpEduLevel,EmpEduName,EmpEduIns,CityName,EmpEduGraduate"
                            + ",EmpEduGrade,EmpEduResult,EmpEduFrontTitle,EmpEduEndTitle FROM employee  "
                            + "join EmpEducations on employee.EmpId = EmpEducations.EmpId "
                            + "join City on City.CityCode = EmpEducations.EmpEduCity "
                            + "WHERE employee.EmpId = {0}";
        var model = new PersonalDataViewModels();

        model.A = db.EmpFamilys.SqlQuery(rtsquery, Convert.ToInt32(@User.Identity.Name)).ToList();
        model.X = await db.Employee.SqlQuery(strquery, Convert.ToInt32(@User.Identity.Name)).FirstOrDefaultAsync();
        model.C = db.EmpEducations.SqlQuery(srtquery, Convert.ToInt32(@User.Identity.Name)).ToList();



        return View(model);

    }


    public ActionResult Details()
    {
        string strquery = "Select employee.EmpId,EmpName,OrgName,EmpJobLvl,JobLvlName,EmpJobTtl,JobTtlName,employee.EmpType,"
                            + "EmpDateStart,EmpEmail,EmpHandphone,EmpJoinDate,EmpSignDate,EmpNPWP,EmpResAddr,city1.CityName AS CityName1,EmpResZipCode, "
                            + "EmpResPhone,EmpResStatus,EmpOriAddr,city2.CityName AS CityName2,EmpOriZipCode,EmpOriPhone,EmpOriStatus,EmpMaritalSt,EmpSex"
                            + " FROM employee "
                            + " join Organization (nolock) on employee.emporg = organization.orgcode "
                            + " join jobtitle (nolock) on employee.empjobttl = jobtitle.jobttlcode "
                            + " join JobLevel (nolock) on employee.EmpJobLvl = JobLevel.JobLvlCode"
                            + " join EmpType (nolock) on employee.EmpType = EmpType.EmpType"
                            + " INNER JOIN City city1 ON employee.EmpResCity = city1.CityCode"
                            + " INNER JOIN City city2 ON employee.EmpOriCity = city2.CityCode"
                            + " WHERE employee.EmpId = {0}";
        string srtquery = "Select employee.EmpID,LvAppId,SubmitDate,LvPeriod,LvDesc,Status,Type,Comment"
                           + " FROM employee  "
                           + "join LeaveApps on employee.EmpId = LeaveApps.EmpId "
                           + "WHERE employee.EmpId = {0}";

        var model = new LeaveApplicationViewModels();
        model.X = db.Employee.SqlQuery(strquery, Convert.ToInt32(@User.Identity.Name)).FirstOrDefault();
        model.C = db.LeaveApps.SqlQuery(srtquery, Convert.ToInt32(@User.Identity.Name)).ToList();

        foreach (var row in model.C)
        {
            string wwwquery = "Select LvAppId,LvDate FROM LeaveDate WHERE LvAppId= {0}";
            row.D = db.LeaveDate.SqlQuery(wwwquery, row.LvAppId).ToList();
        }


        /*info*/
        string sssquery = "Select LeaveDate.LvAppId,LvDate FROM LeaveDate "
                          + " join LeaveApps (nolock) on LeaveDate.LvAppId = LeaveApps.LvAppId "
                          + " WHERE LeaveDate.EmpId= {0} AND Status <> {1} AND Type = {2} ";
        var used =db.LeaveDate.SqlQuery(sssquery, Convert.ToInt32(@User.Identity.Name), "rejected","Annual").ToList().Count;

        string maxquery = "Select Max,LeaveType,ID,TypeDetail FROM LeaveRule WHERE LeaveType='Annual'";
        var max = db.LeaveRule.SqlQuery(maxquery).FirstOrDefault().Max;

        var rem = max - used;
        ViewBag.a = used;
        ViewBag.b= rem;
        ViewBag.c = max;

        /*rule*/
        int comp = 0;
        var pendingtoogle = model.C.Where(x => x.Status == "pending" && x.Type == "Annual").ToList().Count;
        if (used < 12 && pendingtoogle == 0)
        { comp = 1; }
        else {comp = 0; }
        ViewBag.comp = comp;

        return View(model);
    }
    
posta Fimblutterr 01.06.2016 - 13:56
fonte

3 risposte

1

...can you please tell me if I'm doing it right or wrong?

Sfortunatamente, la maggior parte qui probabilmente non sarebbe d'accordo che questo sia il modo giusto per strutturare le cose.

C'è una serie di problemi che riesco a vedere:

Non hai livelli per l'applicazione, il tuo controller popola la vista con risultati che, di per sé, esce dal database. Dividere la logica in livelli, accesso ai dati, regole aziendali, presentazione ... forse altri. Dovresti quindi essere in grado di riutilizzare questi livelli tra i controller e anche tra le applicazioni se li separi nei loro moduli.

Stai scrivendo stringhe SQL in c #. Personalmente, ogni volta che ho un blocco di rosso sul mio schermo in cui un altro approccio potrebbe sostituirlo con un blocco di codice leggibile strutturato e sintatticamente validato, opto per la scala. Utilizzare la struttura del database per creare una procedura che può essere utilizzata per un certo numero di queste operazioni di recupero dei dati rispetto alla stessa serie di tabelle. Oppure puoi usare Entity Framework per permetterti di scrivere l'SQL usando LINQ. Qualunque cosa è meglio allora usando i comandi SQL stringa.

Nome variabile:

string strquery...string rtsquery...string srtquery

Questi sono un problema, chiunque altro non avrà difficoltà a seguire nel tentativo di leggere il codice. Se queste variabili vengono utilizzate o aggiornate in luoghi diversi, ingrandisce la complessità del codice.

Anche questo potrebbe essere un grosso problema:

foreach (var row in model.C)
    {
        string wwwquery = "Select LvAppId,LvDate FROM LeaveDate WHERE LvAppId= {0}";
        row.D = db.LeaveDate.SqlQuery(wwwquery, row.LvAppId).ToList();
    }

Sembra che stiamo chiedendo db per qualcosa per riga in qualsiasi cosa sia model.C . Ciò causerà un problema se model.C decide di avere 1.000 righe.

Refactor che effettua una singola chiamata db con un elenco di LvAppId di

Questo è tutto ciò che ho avuto il cambiamento per guardare davvero, ci potrebbero essere più problemi.

    
risposta data 01.06.2016 - 14:38
fonte
1

prima di tutto puoi spostare il tuo contesto db in una classe separata e preferibilmente proiettare anche. questa classe sarebbe il tuo livello dati, dovrebbe avere tutte le query al suo interno. Li chiameresti come funzioni come questa (che potrebbe essere meglio chiamato per dire quello che stanno ottenendo)
model.A = getStrQueryData(EmpId) model.X = getRtsQueryData(EmpId) model.C = getSrtQueryData(EmpId)
Si potrebbe quindi andare oltre e nella classe del modello aggiungere un costruttore che inserisce i dati quando viene fornito un ID dipendente o utilizzare una funzione separata. il tuo controller sarebbe quindi:
var model = new PersonalDataViewModels(EmpId); return View(model);

o

var model = new PersonalDataViewModels(); model.getPersonalData(EmpId); return View(model);

    
risposta data 01.06.2016 - 14:28
fonte
0

In sostanza ciò che hai sbagliato era copiare / incollare il codice.

NON copiare il codice.

Invece, se vuoi riutilizzare il codice

  • Sposta il codice in una funzione (questo è il motivo per cui sono state inventate le funzioni).
  • Chiama la funzione.

Ci sono opinioni diverse su dove mettere la funzione (modello, livello di servizio, controller, controller di base) ma questa è la ciliegina sulla torta - puoi riordinarlo in un secondo momento.

Esercitati a creare funzioni e riutilizzare prima il codice. Una volta che hai questa abilità, il resto verrà facilmente.

    
risposta data 02.06.2016 - 14:27
fonte

Leggi altre domande sui tag