¿Es esta forma común de construir esta clase y método? -- # camp codereview Relacionados El problema

Is this common way to build this class and method?


3
vote

problema

Español

Soy un nuevo desarrollador, por lo tanto, tengo muchas preguntas:

En esta clase, obtengo la ruta de archivo en el disco (archivo Wireshark que debe convertirse en la extensión de formato PCAP) y convertirla con el método struct Item { let imageName: String let description: String } 0 , en el constructor, estoy llamando struct Item { let imageName: String let description: String } 1 y de la principal Después de crear el objeto EditCAP que estoy recibiendo la nueva ruta de archivo (formato PCAP) con la propiedad struct Item { let imageName: String let description: String } 2 que devuelva struct Item { let imageName: String let description: String } 3 (Miembro de la clase) Y quiero saber si hay una mejor manera o una forma adecuada de hacerlo.

  struct Item {     let imageName: String     let description: String } 4  
Original en ingles

I am a new developer therefore I have many questions:

In this class I get file path on the disk (wireshark file that needs to be converted to pcap format extension) and convert it with the method convertFileToPcap(), in the constructor I am calling convertFileToPcap() and from the main after create the Editcap object I am receiving the new file path (pcap format) with the property getNewFileName() who return _newFileName (class member) and I want to know if there is a better way or appropriate way to do it.

public class Editcap {     #region members     private string _editpcap;     private ProcessStartInfo _editpcapProcess;     private FileInfo _fileInfo;     private string _newFileName;     #endregion      #region c'tor     public Editcap(FileInfo fileinfo)     {         _fileInfo = fileinfo;         _newFileName = "";         convertFileToPcap();     }     #endregion      public void convertFileToPcap()     {         string oldFileExtension = _fileInfo.Extension;         _newFileName = _fileInfo.FullName.Replace(oldFileExtension, "_new") + ".pcap";         _editpcapProcess = new ProcessStartInfo(string.Format("\"{0}\"", _editpcap));         _editpcapProcess.Arguments = (string.Format("{2}{0}{2} -F libpcap {2}{1}{2}", _fileInfo.FullName, _newFileName, "\""));         _editpcapProcess.WindowStyle = ProcessWindowStyle.Hidden;         _editpcapProcess.RedirectStandardOutput = true;         _editpcapProcess.RedirectStandardError = true;         _editpcapProcess.CreateNoWindow = true;         _editpcapProcess.UseShellExecute = false;         _editpcapProcess.ErrorDialog = false;         Process capinfosProcess = Process.Start(_editpcapProcess);         capinfosProcess.WaitForExit();     }      public string getNewFileName()     {         return _newFileName;     } } 
  

Lista de respuestas

6
 
vote

¡Hola y bienvenido a la revisión del código!

  • Me gustaría primero dirigirlo a su oficial de Microsoft nombramiento Directrices para el desarrollo de C #. Tu código va en contra de algunos de ¡Esos, y si son las directrices de su empresa o del equipo, que genial! Si no tiene un estilo oficial, comience con los que vinculé.
  • definitivamente no llame al método convertir del constructor. Esto podría arrojar excepciones que impidan que el objeto fuera creado. Mantenga sus métodos de procesamiento separados y deje que la persona que llama llámalo. Es public , después de todo!
  • Las variables de sus miembros también parecen un poco, bien, globales, en algunos casos. Solo _fileInfo y _newFileName se utilizan en más de un método. Mantenga a los demás locales a sus métodos.
    • Como parte de esto, tenga en cuenta que _editpcap nunca se le asigna y es siempre null . No creo que eso sea lo que está con la intención.
  • El método getNewFileName55544335 se ve muy como Java como Java no tenía El concepto de propiedades. Este es un candidato principal para ser hecho un propiedad.
  • El Process es IDisposable , enviándolo en un using El bloque es idiomático.
  • Ahora solo estoy metiéndome en Nitpicking menor, los campos establecidos en el El constructor es invariante después de la construcción, así que haz la intención. conocido utilizando el readonly palabra clave. Además, puedes emplear C # moderno. Servicios como inicializadores de objetos y el _fileInfo0 palabra clave.

Todo esto que se dice, aquí está mi refactor propuesto del código:

  _fileInfo1  
 

Hello and welcome to Code Review!

  • I'd like to first direct you to Microsoft's official naming guidelines for C# development. Your code goes against a few of those, and if that's your company's or team's guidelines, than great! If you don't have an official style, start with the ones I linked to.
  • I'd definitely not call the convert method from the constructor. This could throw exceptions that would prevent the object from being created. Keep your processing methods separate and let the caller call it. It IS public, after all!
  • your member variables seem a little too, well, global, in some cases. Only _fileInfo and _newFileName are used in more than one method. Keep the others local to their methods.
    • As part of this, note that _editpcap never gets assigned and is always null. I don't think that's what you're intending.
  • the getNewFileName method looks very Java-like as Java didn't have the concept of properties. This is a prime candidate for being made a property.
  • the Process class is IDisposable, so wrapping it in a using block is idiomatic.
  • now I'm just getting into minor nitpicking, the fields set in the constructor are invariant after construction, so make the intent known by using the readonly keyword. Also, you can employ modern C# amenities as object initializers and the var keyword.

All this being said, here is my proposed refactor of the code:

public class Editcap {     #region members     private readonly FileInfo fileInfo;     private string newFileName = string.Empty;     #endregion      #region c'tor     public Editcap(FileInfo fileinfo)     {         if (fileinfo == null)         {             throw new ArgumentNullException("fileInfo");         }          this.fileInfo = fileinfo;     }     #endregion      public void ConvertFileToPcap()     {         this.newFileName = this.fileInfo.FullName.Replace(this.fileInfo.Extension, "_new") + ".pcap";          string editpcap = null; // still not set, need to fix!         var editpcapProcess = new ProcessStartInfo(string.Format("\"{0}\"", editpcap))         {             Arguments = string.Format("{2}{0}{2} -F libpcap {2}{1}{2}", this.fileInfo.FullName, this.newFileName, "\""),             WindowStyle = ProcessWindowStyle.Hidden,             RedirectStandardOutput = true,             RedirectStandardError = true,             CreateNoWindow = true,             UseShellExecute = false,             ErrorDialog = false         };          using (var capinfosProcess = Process.Start(editpcapProcess))         {             capinfosProcess.WaitForExit();         }     }      public string NewFileName     {         get         {             return this.newFileName;         }     } } 
 
 
   
   

Relacionados problema

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...

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...

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...

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...

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 ...

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  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...

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...

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...




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