Pequeño método que ejecuta varios procesos -- # camp codereview Relacionados El problema

Small method that runs various processes


4
vote

problema

Español

Tengo un proyecto que se extiende desde una tabla de DB, su propósito es ejecutar procesos escritos de varias maneras.

Actualmente se ocupa de los scripts de Python, los scripts VBS y los archivos EXE de la consola.

funciona pero me pregunto si hay refactores obvios?

  private Process aProcess;  public void runProcess(string aPath,string aName,string aFiletype) {    aProcess = new Process();   string stInfoFileName = null;   string stInfoArgs = null;   bool blUseShell = false;     //if it is to call a python script   if(aFiletype == "py")   {     stInfoFileName = @"python.exe";     stInfoArgs = string.Format(""{0}{1}{2}"",aPath,Path.DirectorySeparatorChar,aName);   }    //if it is to call console app exe   if(aFiletype == "exe")   {     stInfoFileName = aName;     stInfoArgs = string.Format(""{0}{1}{2}"",aPath,Path.DirectorySeparatorChar,aName);     blUseShell = true;   }    //if it is to run vbs script   if(aFiletype == "vbs")   {     stInfoFileName = "cscript";     stInfoArgs = string.Format("/B "{0}{1}"",@aPath,@aName);   }      aProcess.StartInfo.FileName = stInfoFileName;   aProcess.StartInfo.Arguments = stInfoArgs;    aProcess.StartInfo.WorkingDirectory = aPath;           //<< does not seem to need the extra quotes    aProcess.StartInfo.UseShellExecute = blUseShell;   aProcess.StartInfo.RedirectStandardOutput = false;    aProcess.Start();    this.aProcess.WaitForExit();   this.aProcess.Dispose();  }   
Original en ingles

I have a project which runs from a db table - it's purpose is to run processes written in various ways.

Currently it deals with python scripts, vbs scripts and console exe files.

It works but I'm wondering if there any obvious refactors?

private Process aProcess;  public void runProcess(string aPath,string aName,string aFiletype) {    aProcess = new Process();   string stInfoFileName = null;   string stInfoArgs = null;   bool blUseShell = false;     //if it is to call a python script   if(aFiletype == "py")   {     stInfoFileName = @"python.exe";     stInfoArgs = string.Format("\"{0}{1}{2}\"",aPath,Path.DirectorySeparatorChar,aName);   }    //if it is to call console app exe   if(aFiletype == "exe")   {     stInfoFileName = aName;     stInfoArgs = string.Format("\"{0}{1}{2}\"",aPath,Path.DirectorySeparatorChar,aName);     blUseShell = true;   }    //if it is to run vbs script   if(aFiletype == "vbs")   {     stInfoFileName = "cscript";     stInfoArgs = string.Format("/B \"{0}{1}\"",@aPath,@aName);   }      aProcess.StartInfo.FileName = stInfoFileName;   aProcess.StartInfo.Arguments = stInfoArgs;    aProcess.StartInfo.WorkingDirectory = aPath;           //<< does not seem to need the extra quotes    aProcess.StartInfo.UseShellExecute = blUseShell;   aProcess.StartInfo.RedirectStandardOutput = false;    aProcess.Start();    this.aProcess.WaitForExit();   this.aProcess.Dispose();  } 
  

Lista de respuestas

4
 
vote
vote
La mejor respuesta
 

Primero comenzaría con las convenciones de nombramiento. Los prefijos generalmente se evitan y el nombre del método es Pascalcase:

  list3  

Honestamente, aceptaría solo un parámetro con un camino completo y la dividiría dentro de este método, pero es posible que no puedas cambiar esto, entonces no lo comentaré.

Primero, extraería un método para crear el list4 LEAVINIG En este método solo validación de invocación y parámetros:

  list5  

Luego, moviéndose a list6 Implementación, primero caería esos list7 a favor de un list8 o una lista lisa porque es mucho Más fácil de extender y podría provenir de un archivo de configuración (sin necesidad de cambiar el código para admitir más tipos de archivos).

Por el momento Mantener el IF en el lugar, pero considere:

  • No realice comparación de cadena con list9 . Es una comparación sensible a mayúsculas y receptores de cultura actual, no es obvio y no es probablemente lo que quiera. Use dict0 Especificación del derecho dict1 / dict22 Valor.

  • No construya manualmente las rutas, hay dict3 para eso. Veamos un ejemplo en el que para la simplicidad tengo como ingresar la ruta completa del archivo.

Dado este titular :

  dict4  

Podemos declarar una lista de tipos de archivos conocidos como este (tenga en cuenta que su puede estar fuera del método y estática):

  dict5  

¿Necesita agregar un nuevo tipo de archivo conocido? Añadir una línea aquí. Si esos valores provienen de un archivo de configuración, es posible que desee usar dict6 en su lugar (tenga cuidado si la configuración se puede manipular). Porque no me gustan las secuencias de escape. Introduciría un método de ayuda:

  dict7  

usándolo puedo SIMPLIFICAR ARRIBA CÓDIGO A:

  dict8  

Para crear el dict9 usted solo necesita:

  mtn0  

Tenga en cuenta que aquí usé una matriz simple, pero si opta por un 998877666554433311 (con el comparador insensible del caso adecuado para las teclas), el código puede ser ligeramente simplificado para:

  mtn2  
 

I'd first start with naming conventions. Prefixes are usually avoided and method name is PascalCase:

public void RunProcess(string path, string fileName, string fileExtension) 

Honestly I'd accept just one parameter with full path and I'd split it inside this method but you might not be able to change this then I won't comment about it.

First I'd extract a method to create the StartupInfo leavinig in this method only invocation and parameters validation:

if (path == null)     throw new ArgumentNullException(nameof(path));  // More validations...  using (var process = Process.Start(CreateStartupInfo(path, fileName, fileExtension))) {     process.WaitForExit(); } 

Then, moving to CreateStartupInfo() implementation, I'd first drop those if in favor of a Dictionary or a plain list because it's much easier to extend and it might come from a configuration file (no need to change code to support more file types.)

For the moment keep the if in-place but consider:

  • Do not perform string comparison with ==. It's a current culture aware case sensitive comparison, it's not obvious and it's not probably what you want. Use String.Equals() specifying the right StringComparer/StringComparison value.

  • Do not manually build paths, there is Path.Combine() for that. Let's see an example where for simplicity I have as input the full file path.

Given this holder:

sealed class KnownFileType {     public KnownFileType(string type,         bool useShellExecute,         Func<string, string> interpreter,         Func<string, string> interpreterCommandLine)     {         // ...     }      public string Type { get; }     public Func<string, string> Interpreter { get; }     public Func<string, string> InterpreterCommandLine { get; }     public bool UseShellExecute { get;  } } 

We can declare a list of known file types like this (note that his can be outside the method and static):

var fileTypes = new KnownFileType[] {     new KnownFileType(".py", false, x => "python.exe", x => $"\"{x}\""),     new KnownFileType(".exe", true, x => Path.GetFileName(x), x => $"\"{x}\""),     new KnownFileType(".vbs", false, x => "cscript.exe", x => $"/B \"{x}\""), }; 

Do you need to add new known file type? Add one line here. If those values come from a configuration file then you might want to use String.Format() instead (be careful if configuration can be tampered.) Because I dislike escape sequences I'd introduce an helper method:

private static string Quote(string text) => '"' + text + '"'; 

Using it I may simplify above code to:

new KnownFileType(".py", false, x => "python.exe", x => Quote(x)) 

To create the ProcessStartupInfo you just need to:

string extension = Path.GetExtension(fullPath); var fileType = fileTypes.First(x => String.Equals(x.Type, extension, StringComparison.InvariantCultureIgnoreCase));  return new ProcessStartInfo {     UseShellExecute = fileType.UseShellExecute,     FileName = fileType.Interpreter(fullPath),     Arguments = fileType.InterpreterCommandLine(fullPath),     WorkingDirectory = Path.GetDirectoryName(fullPath) }; 

Note that here I used a plain array but if you opt for a Dictionary<string, KnownFileType> (with the proper case insensitive comparer for keys) then code may be slightly simplified to:

var fileType = fileTypes[Path.GetExtension(fullPath)]; // ... 
 
 
4
 
vote

Antes de incluso considerar los refactores, creo que hay un error.

  mtn3  

¿Por qué llamar mtn4 ? Puedo pensar en muy algunas situaciones en las que un EXE debe tomar su propio nombre como un argumento de línea de comandos.


También estoy inconvincado por la estructura general. ¿Por qué su programa debería tener que ocuparse de los tipos de archivos de mapeo a los ejecutores? Esa es la funcionalidad que está integrada en el sistema operativo. Si el registro está configurado correctamente, el método debe ser solo

  mtn5  
 

Before even considering refactors, I think there's a bug.

  //if it is to call console app exe   if(aFiletype == "exe")   {     stInfoFileName = aName;     stInfoArgs = string.Format("\"{0}{1}{2}\"",aPath,Path.DirectorySeparatorChar,aName); 

Why call foo.exe "path\foo.exe"? I can think of very few situations in which an exe should take its own name as a command-line argument.


I'm also unconvinced by the general structure. Why should your program have to take care of mapping file types to executors? That's functionality that's built into the operating system. If the registry is correctly configured then the method should just be

public void RunProcess(string aPath, string aName, string aFiletype) {   using (var aProcess = new Process())   {     aProcess.StartInfo.FileName = Path.Combine(aPath, aName);     aProcess.StartInfo.WorkingDirectory = aPath;     aProcess.StartInfo.UseShellExecute = true;     aProcess.Start();     aProcess.WaitForExit();   } } 
 
 
   
   
2
 
vote

Algo más que puede aprovecharse de la interpolación de cadenas, pero eso depende de qué versión de C # que está utilizando actualmente.

Al usar la interpolación de cadenas, puede deshacerse de la llamada a String.Format y reemplace los marcadores de posición con el objeto en realidad que desea insertar en la cadena.

este

    if(aFiletype == "py")   {     stInfoFileName = @"python.exe";     stInfoArgs = string.Format(""{0}{1}{2}"",aPath,Path.DirectorySeparatorChar,aName);   }   

se convierte en este

  if(aFiletype == "py") {     stInfoFileName = @"python.exe";     stInfoArgs = $"{aPath}{Path.DirectorySeparatorChar}{aName}"; }   

que se ve mucho más limpio, en mi opinión.


También noté que está llamando a un método 99887766655544333 9988776655544334 implementa el 9988776655544335 interfaz y esta interfaz le permite usar una using Block . Puede evitar que cierre la aplicación de la misma manera que lo está haciendo actualmente llamando al método 9988776665544337 using .

El beneficio de usar un bloque using es que incluso si hay un error que se eliminará el proceso y la memoria será liberada.

Tras una investigación adicional, parece que el Clase de proceso hereda el Clase de componentes que implementa Interfaz iDisposición .

 

Something else that you may be able to take advantage of is String Interpolation, but that depends on which version of C# you are currently using.

By using String Interpolation you can get rid of the call to String.Format and replace the placeholders with the actually object that you want inserted into the string.

This

  if(aFiletype == "py")   {     stInfoFileName = @"python.exe";     stInfoArgs = string.Format("\"{0}{1}{2}\"",aPath,Path.DirectorySeparatorChar,aName);   } 

Becomes this

if(aFiletype == "py") {     stInfoFileName = @"python.exe";     stInfoArgs = $"{aPath}{Path.DirectorySeparatorChar}{aName}"; } 

Which looks a lot cleaner, in my opinion.


I also noticed that you are calling a .Dispose() Method which tells me the Process object implements the IDisposable interface and this interface allows you to use a using statement block. You can keep it from closing the application in the same way that you are currently doing it by calling the WaitForExit() method as the last command before closing the using block.

The benefit of using a using block is that even if there is an error the process will be disposed of and the memory will be released.

Upon further research it appears that the Process Class Inherits the Component Class which implements the IDisposable interface.

 
 
   
   
1
 
vote

Lo primero que aprobaría en este Código: su extensibilidad. Actualmente, cada vez que agrega soporte al nuevo tipo de archivo, debe abrir el código fuente de su solicitud, modificarlo, compilarlo y solo, puede usarlo. Lo mismo ocurre con la modificación del corredor asociado con el tipo de archivo o cambie el formato de ARGS que pasan a procesos. Hice esta parte configurable sobre la marcha. P.ej. Con XML o archivo de configuración JSON:

    if(aFiletype == "py")   {     stInfoFileName = @"python.exe";     stInfoArgs = string.Format(""{0}{1}{2}"",aPath,Path.DirectorySeparatorChar,aName);   } 0  

Tenga en cuenta que he agregado soporte de varias extensiones a un solo corredor. Luego crearía clase de corredor que usaría todas estas configuraciones:

    if(aFiletype == "py")   {     stInfoFileName = @"python.exe";     stInfoArgs = string.Format(""{0}{1}{2}"",aPath,Path.DirectorySeparatorChar,aName);   } 1  

También puede agregar alguna validación para estas propiedades, o crearlas como solo lectura (asignando valores a través del constructor). Entonces, tenemos todos los datos en su lugar. Solo queda el comportamiento

    if(aFiletype == "py")   {     stInfoFileName = @"python.exe";     stInfoArgs = string.Format(""{0}{1}{2}"",aPath,Path.DirectorySeparatorChar,aName);   } 2  

simple como eso. Ahora necesitamos a alguien que cree corredores y los seleccione de acuerdo con el tipo de archivo pasado. Puede crear un archivo XXXConfiguración XXX específico, pero creo que está bien crear corredores directamente desde XML

    if(aFiletype == "py")   {     stInfoFileName = @"python.exe";     stInfoArgs = string.Format(""{0}{1}{2}"",aPath,Path.DirectorySeparatorChar,aName);   } 3  

Ahora prepare el diccionario con el mapeo de FileType-Runner (puede agregar validación aquí para verificar si algunos tipos de archivos se asignan a varios corredores):

    if(aFiletype == "py")   {     stInfoFileName = @"python.exe";     stInfoArgs = string.Format(""{0}{1}{2}"",aPath,Path.DirectorySeparatorChar,aName);   } 4  

y ejecutar sus archivos se verá como:

    if(aFiletype == "py")   {     stInfoFileName = @"python.exe";     stInfoArgs = string.Format(""{0}{1}{2}"",aPath,Path.DirectorySeparatorChar,aName);   } 5  
 

The first thing which I would approve in this code - it's extensibility. Currently each time you add support to new file type, you have to open source code of your application, modify it, compile and only then you can use it. Same goes to modifying runner associated with file type, or changing format of args passed to process. I would made this part configurable on the fly. E.g. with xml or json config file:

<runners>  <runner fileTypes=".py" path="python.exe" argsFormat="${dir}\${file}" />  <runner fileTypes=".exe|.bat" path="${file}" argsFormat="${dir}\${file}" useShell="true"/>  <runner fileTypes=".vbs" path="cscript" argsFormat="/B &quot;{dir}{file}&quot;"/> </runners> 

Note that I have added support of several extensions to single runner. Then I would create runner class which would use all these settings:

public class ProcessRunner {     public IEnumerable<string> SupportedExtensions { get; set; }     public string RunnerPath { get; set; }     public bool UseShell { get; set; }     public string ArgsFormat { get; set; }      // behavior goes here  } 

You can also add some validation for these properties, or create them as read-only (assigning values via constructor). So, we have all data in place. Only behavior is left

public void Run(string path) {     var fileExtension = Path.GetExtension(path);     var comparer = StringComparer.InvariantCultureIgnoreCase;     if (!SupportedExtensions.Contains(fileExtension, comparer))         throw new NotSupportedException($"File type {fileExtension} is not supported.");      using (Process process = new Process())     {         process.StartInfo = BuildStartInfo(path);         process.Start();         process.WaitForExit();     } }  private ProcessStartInfo BuildStartInfo(string path) {     var dir = Path.GetDirectoryName(path);     var file = Path.GetFileName(path);     var keywords = new Keywords().Add("dir", dir).Add("file", file);      return new ProcessStartInfo     {         FileName = keywords.Replace(RunnerPath),         Arguments = keywords.Replace(ArgsFormat),         WorkingDirectory = dir,         UseShellExecute = UseShell,         RedirectStandardOutput = false     }; } 

Simple as that. Now we need someone to create runners and pick them according to file type passed. You can build some specific xxxConfiguration file, but I believe it's OK to create runners directly from xml

var configFile = "runner.config"; var runners = from r in XDocument.Load(configFile).Root.Elements("runner")               select new ProcessRunner               {                   RunnerPath = (string)r.Attribute("path"),                   SupportedExtensions = r.Attribute("fileTypes")?.Value                      .Split(new[] { '|' }, StringSplitOptions.RemoveEmptyEntries),                   ArgsFormat = (string)r.Attribute("argsFormat"),                   UseShell = (bool?)r.Attribute("useShell") ?? false               }; 

Now prepare dictionary with fileType-runner mapping (you can add validation here as well to check whether some fileTypes are mapped to several runners):

var runnersByFileType = runners   .SelectMany(r => r.SupportedExtensions, (runner, fileType) => new { runner, fileType })   .ToDictionary(x => x.fileType, x => x.runner, StringComparer.InvariantCultureIgnoreCase); 

And running your files will look like:

ProcessRunner runner; var fileExtension = Path.GetExtension(path); if (!runnersByFileType.TryGetValue(fileExtension, out runner))         // handle not supported fileType case  runner.Run(path); 
 
 
 
 

Relacionados problema

35  Demasiados bucles en la aplicación de dibujo  ( Too many loops in drawing app ) 
Tengo un método que tiene muchos bucles: #ifndef __RUNES_STRUCTURES_H #define __RUNES_STRUCTURES_H /* Runes structures. */ struct Game { char board[2...

7  Versión LINQ del algoritmo de recocido simulado  ( Linq version of the simulated annealing algorithm ) 
Decidí intentarlo e implementar (una versión de) la recocido simulado algoritmo usando solo linq , solo para ver si pudiera. Me encantaría si alguien pudi...

3  Mientras que el bucle usa variables adicionales, ¿se puede limpiar esto?  ( While loop uses extra variables can this be cleaned up ) 
Una pieza de mi programa permite que se utilice el bucle y la incremento de una entidad seleccionada en un bucle de tiempo en otro lugar. Aquí hay una muestra...

7  Colecciones vacías en caché  ( Cached empty collections ) 
A menudo necesito devolver las colecciones vacías. Uno de esos días, escribí lo siguiente para devolver una instancia en caché: public static class Array<...

3  Generador de imágenes de Mandelbrot con iteración paralela  ( Mandelbrot image generator with parallel iteration ) 
Actualmente estoy tratando de optimizar esta clase que tengo para la generación fractal. La ecuación está destinada a ser conectable; He usado z => z*z + c ...

0  Creación de múltiples objetos del extracto de SQL Server  ( Creating multiple objects from sql server extract ) 
He creado una solución prototipo simplificada como una especie de prueba de concepto antes de comenzar un programa más grande. Aquí están los datos de prueb...

3  Genéricos anulables - implementando secuencialSearchst en C #  ( Nullable generics implementing sequentialsearchst in c ) 
Para fines de aprendizaje, estoy implementando cierto código de SEDEDWICK & AMP; Los algoritmos de Wayne, cuarta edición . Debido a las características del...

6  PRIGO DE PODER TICTACTOO EN C #  ( Command prompt tictactoe in c ) 
Escribí un juego básico de comando TIC TAC TOE juego. Quiero saber qué se puede mejorar en términos de modelado y qué errores he hecho (si corresponde). vo...

6  ¿Está mi delegado definió la forma correcta y necesita transformarse en una mariposa bonita?  ( Is my delegate defined the right way and does it need to transform to a pretty e ) 
He leído tantos Historias de tiempo de cama e inserciones de cómo < fuertes> delegados trabajo y por qué eventos deben ser reemplazados por los delegado...

1  TPL HILO FUGA Y FUGA DE MEMORIA  ( Tpl thread leak and memory leak ) 
Estoy tratando de rastrear lo que supongo que debo ser una pérdida de memoria / hilo en uno de mis programas. Mi programa usa una función para cargar un arc...




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