¿Cómo podría hacer que mi controlador sea un poco menos horrible? -- # campo con mvc campo con controller campo con asp.net-mvc-4 camp codereview Relacionados El problema

How could I make my controller a little less hideous?


0
vote

problema

Español

Tengo un controlador. Funciona de la siguiente manera:

Un usuario importa un archivo y decide si los contenidos de los archivos contienen nuevas entradas que se colocarán en una base de datos, o las entradas de entradas existentes se actualizarán.

El usuario hace clic en el botón Importar y se envía al controlador a continuación. Si hay advertencias o errores, se muestra la pantalla de resultados de la validación: para ayudar a los usuarios a corregir sus datos.

Si el archivo no contiene errores o advertencias, la pantalla de validación se omitirá y se insertará sus contenidos de archivos en la base de datos.

Esta es claramente demasiado lógica para un controlador, ya que he leído que el controlador simplemente está destinado a decidir qué mirada enviar los datos a. Estoy teniendo un tiempo bastante difícil determinar cómo extraer los datos.

Estoy buscando sugerencias sobre cuál debería ser mi siguiente paso. Gracias a cualquiera que lleve tiempo para ayudarme.

  public ActionResult ValidationResultsScreen(HttpPostedFileBase file, DatabaseAccessMode databaseAccess) {     try     {         var fileLocation = Path.Combine(Path.GetTempPath(), string.Join(Guid.NewGuid().ToString(), file.FileName));          Session["TempFileLocation"] = fileLocation;         Session["DatabaseAccessMode"] = databaseAccess;          file.SaveAs(fileLocation);          var importer= new Importer(_connectionString, _logManager, databaseAccess,Server.MapPath("..\bin"));          var parsedValues= importer.ParseFile(fileLocation);          var failedValues = ParsedValues.Where(x => x.ParsedState != ParsedState.Success);          if (failedValues.Any())//if there were any warnings or errors display the validation results screen         {             return View(processedValues);         }         //else import the file to the database          var transactionResults = importer.UploadToDatabase(parsedValues);          var failedDBTransactions= RetrieveFailedResults(transactionResults);          return failedDBTransactions.Any()              ? View("DatabaseImportErrorScreen", failedDBTransactions)              : View("DatabaseImportResults", transactionResults);     }     catch (Exception exception)     {         ViewBag.ExceptionMessage = exception.Message;         return View("DefaultErrorScreen");     } }   
Original en ingles

I have a controller. It functions as follows:

A user imports a file and decides whether the files contents contain new entries to be placed into a database, or existing entries entries to be updated.

The user clicks on the import button and is sent to the controller below. If there are any warnings or errors, the validation results screen is shown - to help the users correct their data.

If the file doesn't contain any errors or warnings, the validation screen will be skipped and their file contents will be inserted/updated into the database.

This is clearly far too much logic for a controller as I have read that the controller is simply meant to decide which view to send the data to. I am having a rather difficult time determining how to extract out the data.

I am looking for suggestions on what my next step should be. Thank you to anyone who takes time to help me.

public ActionResult ValidationResultsScreen(HttpPostedFileBase file, DatabaseAccessMode databaseAccess) {     try     {         var fileLocation = Path.Combine(Path.GetTempPath(), string.Join(Guid.NewGuid().ToString(), file.FileName));          Session["TempFileLocation"] = fileLocation;         Session["DatabaseAccessMode"] = databaseAccess;          file.SaveAs(fileLocation);          var importer= new Importer(_connectionString, _logManager, databaseAccess,Server.MapPath("..\\bin"));          var parsedValues= importer.ParseFile(fileLocation);          var failedValues = ParsedValues.Where(x => x.ParsedState != ParsedState.Success);          if (failedValues.Any())//if there were any warnings or errors display the validation results screen         {             return View(processedValues);         }         //else import the file to the database          var transactionResults = importer.UploadToDatabase(parsedValues);          var failedDBTransactions= RetrieveFailedResults(transactionResults);          return failedDBTransactions.Any()              ? View("DatabaseImportErrorScreen", failedDBTransactions)              : View("DatabaseImportResults", transactionResults);     }     catch (Exception exception)     {         ViewBag.ExceptionMessage = exception.Message;         return View("DefaultErrorScreen");     } } 
           
         
         

Lista de respuestas

1
 
vote
vote
La mejor respuesta
 

tal vez algo como esto? (Comentario incluido):

  JSON.stringify1  

Sin embargo, es posible que haya notado que eliminé la utilización de un objeto de sesión

Podría ver algún valor para tener un modo de acceso a la base de datos en sesión. Aun así, el uso de una sesión a los datos de caché puede causar una multitud de problemas si no tiene una página de tiempo de espera de sesión o formas de rellenar su sesión en el tiempo de espera de la sesión. Además, usar la sesión directamente podría introducir problemas de carrera.

Un intento de seguir usando la sesión mientras evita los problemas de la carrera puede ser lo siguiente:

  JSON.stringify2  

Pero si tiene un conjunto completo de estos valores, y cambiar cualquiera de ellos puede invalidar a todos los demás, probablemente sería mejor aislar esa lógica en otro lugar.

 

Maybe something like this? (commentary included):

//Do file locations really need to be saved in the session? I would hope not. protected string TempFileLocation {get; set;} //This assumes DatabaseAccessMode is an class of some kind. protected DatabaseAccessMode DatabaseAccess {get;set;}  //place wherever your private methods go... private WhateverTypeORInterfaceParsedValuesIs GetParsedValues(HttpPostedFileBase file) {         TempFileLocation  = Path.Combine(Path.GetTempPath(),              string.Join(Guid.NewGuid().ToString(),              file.FileName));          file.SaveAs(TempFileLocation);         var importer= new Importer(_connectionString,              _logManager,              DatabaseAccess ,Server.MapPath("..\\bin"));         var parsedValues= importer.ParseFile(TempFileLocation);          return parsedValues; }  //and of course, your new Action public ActionResult ValidationResultsScreen(HttpPostedFileBase file, DatabaseAccessMode databaseAccess) {     try     {         //save it for later.          //(we still need to figure out a proper lifetime         // for your Database Access mode)         DatabaseAccess = databaseAccess;           //grab parsed values.         var parsedValues = GetParsedValues(file)          //if there were any warnings or errors display the validation results screen         if (parsedValues.Where(x => x.ParsedState != ParsedState.Success).Any())         {             return View(processedValues); // what are the processedValues???         }          else         {              //import the file to the database            var transactionResults = importer.UploadToDatabase(parsedValues);            var failedDBTransactions= RetrieveFailedResults(transactionResults);             return failedDBTransactions.Any()              ? View("DatabaseImportErrorScreen", failedDBTransactions)              : View("DatabaseImportResults", transactionResults);         }     }     catch (Exception exception)     {         ViewBag.ExceptionMessage = exception.Message;         return View("DefaultErrorScreen");     } } 

However, you may have noticed I removed the utilization of a Session object

I could see some value to having a Database access mode in session. Even so, using a session to cache data can cause a multitude of problems if you don't have a session timeout page or ways to refill your session on session timeout. Also, using the session directly might introduce race issues.

One attempt to still use the session while avoiding race issues might be the following:

private DatabaseAccessMode _DataAccess; protected DatabaseAccessMode DatabaseAccess {      get{          //save the first non-null value retrieved and use that every time.           _DataAccess = _DataAccess                ?? (DatabaseAccessMode) HttpContext.Current.Session["DatabaseAccessMode"]          return _DataAccess ;      }      set{          _DataAccess = value;          HttpContext.Current.Session["DatabaseAccessMode"] = value;      } } 

but if you have a whole set of these values, and changing any one of them might invalidate all the others, it would probably be best to isolate that logic somewhere else.

 
 
       
       
2
 
vote

Según su caso, su controlador parece estar mezclando varias cosas al mismo tiempo. El contexto total no está claro, pero parece que este es un método de controlador en un controlador que hace más que esto, correcto?

En general, sugeriría verlo diferente. Una importación es un objeto, así que cree un controlador separado para ello: ImportScontroller. Que puede ser un controlador muy estándar:

Agregar: usuario Agregar un nuevo archivo, el controlador solo lo envía al modelo de importación - & gt; ahorrar. El modelo lo guarda en un archivo temporal o lo que sea necesario. Puede implementar la validación aquí ya, pero también puede decir: "Un archivo de carga y guardado está bien. Esa es una creación correcta de una importación".

Pseudo Código:

  if(dataIncoming) {     this->Import->save(data)     //handle file save errors, no data received etc. All coming back from model which should take care of the validation.     //redirect to edit }   

Editar: Siguiente Paso es revisar la importación. Aquí puede pedirle al modelo que validate.

Pseudo Código:

  ON SAVE (if this is requested without a save/submit action just show data and validation results) this->Import->parseFile(data) //validate  //show parse file issues to user if any return View(processedValues);  if(no errors in parsefile) {   //try database save   Import->saveInDatabase   //show validation errors if needed    //if all ok you can redirect (or have user interaction) to delete }   

Ver: siempre le muestra los datos de la importación

Eliminar: se puede usar para eliminar una carga de importación incorrecta (archivo cargado de usuario dos veces) o cuando se realiza la importación. Puedes validar esto.

Aquí puede implementar también un eliminación suave, por ejemplo.

Pseudo Código:

  Import->delete   

Entonces, a medida que veo, movería todo ese archivo, TMPFile y otras partes al modelo al ver una importación como un objeto independiente. Eso hace posible tener todos los encapsulados y también bien testibles. Un método de controlador que hace todos los pasos es mucho más difícil de probar. Por ejemplo, necesitaría tomar todas las vistas diferentes renderizadas, etc. Prefiero dividirla.

 

Based on your case your controller seems to be mixing multiple things at the same time. The total context is not clear but it seems this is a controller method in a controller that does more than this, correct?

In general I would suggest to see it different. An import is an object, so create a seperate controller for it: ImportsController. That can be a very standard controller:

Add: User add a new file, controller just sends it to Import model -> save. The model saves it in a temp file or whatever it needs to do. You can implement validation here already but you can also say: "A upload and saved file is ok. That is a correct creation of an import."

Pseudo code:

if(dataIncoming) {     this->Import->save(data)     //handle file save errors, no data received etc. All coming back from model which should take care of the validation.     //redirect to edit } 

Edit: Next step is to review the import. Here you can ask the model to validate.

Pseudo code:

ON SAVE (if this is requested without a save/submit action just show data and validation results) this->Import->parseFile(data) //validate  //show parse file issues to user if any return View(processedValues);  if(no errors in parsefile) {   //try database save   Import->saveInDatabase   //show validation errors if needed    //if all ok you can redirect (or have user interaction) to delete } 

View: Always shows you the data of the import

Delete: Can be used to remove a wrong import upload (user uploaded file twice) or when import is done. You can validate this.

Here you can implement also a soft delete for example.

Pseudo code:

Import->delete 

So as you see I would move all that file, tmpfile and other parts to the model by seeing an import as a standalone object. That makes it possible to have all encapsulated and also well testable. A controller method doing all steps is much harder to test. For example you would need to catch all different views rendered etc. I prefer to split it up.

 
 
   
   

Relacionados problema

1  Creando modelos manualmente en MVC  ( Creating models manually in mvc ) 
Estoy creando mis modelos manualmente. Tengo cinco mesas en mi base de datos de la siguiente manera: Members MemberTypeMasters Payments Relati...

4  Utilizando un sistema recurrente.Trading.Timer en una aplicación MVC  ( Using a recurring system threading timer in an mvc application ) 
Me gustaría su opinión sobre algo. Tengo la siguiente clase: public class ApplicationClock { void tick(object _) { lock (tickLock) ...

3  Actualización de datos relacionados el MVC / EF WAY  ( Updating related data the mvc ef way ) 
Tengo un controlador MVC ASP.NET, las partes relevantes cuyas aparecen aquí: public class IncidentController : Controller { private IncidentDBContext d...

3  Unidad de trabajo MVC4 con marco de entidades  ( Mvc4 unit of work with entity framework ) 
Estoy diseñando una nueva aplicación ASP.NET MVC4. Sé que la clase de contexto generada por EF es unidad de trabajo y DBSET es la clase de repositorio. Solo e...

3  Recuperación de estadísticas sobre clics de URL  ( Retrieving statistics about url clicks ) 
Soy bastante nuevo para ASP / MVC, pero he tenido alguna experiencia de programación previa. Estoy tratando de recuperar estadísticas sobre clics de URL: cl...

6  Autorización PBKDF2  ( Pbkdf2 authorization ) 
He descubierto que el uso de contraseñas hashed con sales es una idea mucho mejor que MD5 / SHA256, por lo que no los estoy pulsando con PBKDF2. Sin embargo, ...

11  Inyectar objetos seleccionados en ViewData para habilitar el uso de EditorFor en las propiedades basadas en desplegables  ( Injecting selectlist objects into viewdata to enable using editorfor on dropdown ) 
He desarrollado un sistema mediante el cual utilizo un atributo para indicar qué <table> <tr> <th colspan="2">Title01</th> </tr> <tr> ...

5  Unidad de trabajo con patrón de repositorio MVC 5 & EF 6  ( Unit of work with repository pattern mvc 5 ef 6 ) 
Archivo una muestra de cómo estoy usando la unidad de trabajo y amplificador; Patrón de repositorio basado en mi comprensión. ¿Alguien puede avisarme si estoy...

3  Envoltura de galletas en MVC4  ( Cookie wrapper in mvc4 ) 
Me gustaría crear una envoltura de cookies en mi solicitud para ayudar a respaldar un enfoque seco para codificar métodos comunes de cookies, proporcione Inte...

4  Devolver valores existentes dentro de la base de datos  ( Return existing values inside database ) 
Aquí hay un método dentro de mi controlador que lee los valores de los datos de ángulo y puntos de mi base de datos. Luego, agarra los datos y lo agrega a una...




© 2022 respuesta.top Reservados todos los derechos. Centro de preguntas y respuestas reservados todos los derechos