Filtrando la conversación por tipo de filtro y valor -- java camp codereview Relacionados El problema

Filtering conversation by filter type and value


5
vote

problema

Español

Me han asignado para desarrollar una característica que filtra una conversación. Por ejemplo, quiero filtrar la conversación por la ID de usuario, luego expórtelo a algo JSON o archivo de texto. En este caso, creé una clase que maneja los filtros como este.

  import java.util.ArrayList; import java.util.List;  /**  * Represents the filter that operates the filter functions for messages  *  * @author Muhammad  */ public class Filter {      String filterType;     String argumentValue;      //Constructor that takes a parameter of filter type and argument value.     public Filter(String filterType, String argumentValue) {         this.filterType = filterType;         this.argumentValue = argumentValue;     }      //Method that filters a conversation by a specific user and return filterd conversation.     public static Conversation filterByUser(Conversation conversation, String specificUser) {         List<Message> messageList = new ArrayList<>();         //Filter by used id         for (Message message : conversation.messages) {             if (message.senderId.equals(specificUser)) {                 messageList.add(message);                 Conversation filteredConversation = new Conversation(conversation.name, messageList);                 conversation = filteredConversation;             }         }         return conversation;     }      //Method that filters a conversation that contains a specific keywod and returns filterd conversation.     public static Conversation filterByWord(Conversation conversation, String specificWord) {         List<Message> messageList = new ArrayList<>();         //Filter by keyword         for (Message message : conversation.messages) {             if (message.content.contains(specificWord)) {                 messageList.add(message);                 Conversation filteredConversation = new Conversation(conversation.name, messageList);                 conversation = filteredConversation;             }         }         return conversation;     }      //Method that hides a word in a conversation by a specificword     public static Conversation hideWord(Conversation conversation, String specificWord) {         List<Message> messageList = new ArrayList<>();         //Filter by used id         for (Message message : conversation.messages) {             if (message.content.contains(specificWord)) {                 message.content = message.content.replaceAll(specificWord, "*redacted*");                 messageList.add(message);                 Conversation filteredConversation = new Conversation(conversation.name, messageList);                 conversation = filteredConversation;             }         }         return conversation;     } }   

En otra clase, la usé dentro de un método llamado filtro como este.

  private void filter(Filter filter, Conversation conversation, String outputFilePath) throws Exception {          String filterType = filter.filterType; //used to get the type of filter         String argumentValue = filter.argumentValue;          //Filterers         switch (filterType) {             case "filteruser":                 conversation = Filter.filterByUser(conversation, argumentValue);                 this.writeConversation(conversation, outputFilePath);                 break;             case "filterword":                 conversation = Filter.filterByWord(conversation, argumentValue);                 this.writeConversation(conversation, outputFilePath);                 break;             case "hideword":                 conversation = Filter.hideWord(conversation, argumentValue);                 this.writeConversation(conversation, outputFilePath);                 break;             default:                 this.writeConversation(conversation, outputFilePath);                 break;         }      }   

Este código funciona, sin embargo, me gustaría comentarios de todos modos, puedo mejorar el código, ya que soy solo un graduado.

Original en ingles

I have been assigned to developed a feature that filters a conversation. For example, I want to filter the conversation by user id then export it to something either JSON or text file. In this case, I created a class that handles the filters like this.

import java.util.ArrayList; import java.util.List;  /**  * Represents the filter that operates the filter functions for messages  *  * @author Muhammad  */ public class Filter {      String filterType;     String argumentValue;      //Constructor that takes a parameter of filter type and argument value.     public Filter(String filterType, String argumentValue) {         this.filterType = filterType;         this.argumentValue = argumentValue;     }      //Method that filters a conversation by a specific user and return filterd conversation.     public static Conversation filterByUser(Conversation conversation, String specificUser) {         List<Message> messageList = new ArrayList<>();         //Filter by used id         for (Message message : conversation.messages) {             if (message.senderId.equals(specificUser)) {                 messageList.add(message);                 Conversation filteredConversation = new Conversation(conversation.name, messageList);                 conversation = filteredConversation;             }         }         return conversation;     }      //Method that filters a conversation that contains a specific keywod and returns filterd conversation.     public static Conversation filterByWord(Conversation conversation, String specificWord) {         List<Message> messageList = new ArrayList<>();         //Filter by keyword         for (Message message : conversation.messages) {             if (message.content.contains(specificWord)) {                 messageList.add(message);                 Conversation filteredConversation = new Conversation(conversation.name, messageList);                 conversation = filteredConversation;             }         }         return conversation;     }      //Method that hides a word in a conversation by a specificword     public static Conversation hideWord(Conversation conversation, String specificWord) {         List<Message> messageList = new ArrayList<>();         //Filter by used id         for (Message message : conversation.messages) {             if (message.content.contains(specificWord)) {                 message.content = message.content.replaceAll(specificWord, "*redacted*");                 messageList.add(message);                 Conversation filteredConversation = new Conversation(conversation.name, messageList);                 conversation = filteredConversation;             }         }         return conversation;     } } 

In another class, I used it inside a method called filter like this.

private void filter(Filter filter, Conversation conversation, String outputFilePath) throws Exception {          String filterType = filter.filterType; //used to get the type of filter         String argumentValue = filter.argumentValue;          //Filterers         switch (filterType) {             case "filteruser":                 conversation = Filter.filterByUser(conversation, argumentValue);                 this.writeConversation(conversation, outputFilePath);                 break;             case "filterword":                 conversation = Filter.filterByWord(conversation, argumentValue);                 this.writeConversation(conversation, outputFilePath);                 break;             case "hideword":                 conversation = Filter.hideWord(conversation, argumentValue);                 this.writeConversation(conversation, outputFilePath);                 break;             default:                 this.writeConversation(conversation, outputFilePath);                 break;         }      } 

This code works, however I'd like feedback on anyways that I can improve the code as I am just a graduate.

  

Lista de respuestas

4
 
vote
vote
La mejor respuesta
 

quiero mencionar solo una cosa especialmente:

Estás usando "Cadena Mágica" para elegir los tipos de filtros. En lugar de hacer eso (porque es frágil), es probable que esté mejor con un enumio:

  {     question:      choices:      correctAnswer:  } 4  

Tu código tendría que ajustar un poco, pero para dar una mirada corta en el método del filtro:

  {     question:      choices:      correctAnswer:  } 5  

Esto expone otra pequeña posibilidad de mejora en su código.

En todos los casos, llamará { question: choices: correctAnswer: } 6 con exactamente los mismos argumentos. Puede moverlo a fuera del bloque de la caja del interruptor:

  {     question:      choices:      correctAnswer:  } 7  

Otro Lo último que quiero recomendar es usar { question: choices: correctAnswer: } 8 INTERMIENTE DE { question: choices: correctAnswer: } 9 Para referirse a function QuizQuesiton(question, choices, correctAnswer) { this.question = question; this.choices = choices; this.correctAnswer = correctAnswer; } 0 . Esto lo hace descaradamente obvio, que realmente se refiere a un archivo. Las cuerdas son ... no rutas ...

 

I want to mention only one thing especially:

You're using "magic string" to choose your filter types. Instead of doing that (because it's brittle) you are probably better off with an enum:

public enum FilterType {     USER, WORD, HIDE_WORD } 

Your code would have to adjust a little, but to give a short look into the filter method:

private void filter(Filter filter, Conversation conversation, String outputFilePath) {     switch (filter.filterType) {         case FilterType.USER:             conversation = Filter.filterByUser(conversation, filter.argumentValue);             writeConversation(conversation, outputFilePath);             break;         case FilterType.WORD:             conversation = Filter.filterByWord(conversation filter.argumentValue);             writeConversation(conversation, outputFilePath);             break;         // ... 

This exposes another small improvement possibility in your code.

In every case you will call this.writeConversation with exactly the same arguments. You can move that to outside the switch-case block:

switch (filter.filterType) {     // ... } this.writeConversation(conversation, outputFilePath); 

Another last thing I want to recommend is using Path intead of String to refer to outputFilePath. This makes it blatantly obvious, that you're actually referring to a File. Strings are .. not Paths...

 
 
4
 
vote

Como motivado por @ Vogel612, publico mi sugerencia como respuesta. Una respuesta probablemente debería contener un poco más que solo un enlace a la patrón de estrategia Adept el patrón a su código presentado.

En lugar de mantener la lógica en la clase function QuizQuesiton(question, choices, correctAnswer) { this.question = question; this.choices = choices; this.correctAnswer = correctAnswer; } 6655443321 , esta clase se refactora a una interfaz que proporciona un método simple 9988777655554433222 .

  function QuizQuesiton(question, choices, correctAnswer) {     this.question = question;     this.choices = choices;     this.correctAnswer = correctAnswer; } 3  

Si necesita cierta lógica base, también podría usar una clase base abstracta en lugar de una interfaz, por supuesto.

Una implementación concreta de un filtro ahora podría parecer esto:

  function QuizQuesiton(question, choices, correctAnswer) {     this.question = question;     this.choices = choices;     this.correctAnswer = correctAnswer; } 4  

Esto le permite refactar el método function QuizQuesiton(question, choices, correctAnswer) { this.question = question; this.choices = choices; this.correctAnswer = correctAnswer; } 5 a algo así:

  function QuizQuesiton(question, choices, correctAnswer) {     this.question = question;     this.choices = choices;     this.correctAnswer = correctAnswer; } 6  

Esto también hace que el function QuizQuesiton(question, choices, correctAnswer) { this.question = question; this.choices = choices; this.correctAnswer = correctAnswer; } 77 sea redundante que, por lo tanto, se puede eliminar. Sin embargo, en lugar de mantener una clase única 99887766655443328 99887766655443329 , no puedo dar más código en el lugar donde el objeto de concreto debe ser instanciado.

Una ventaja del patrón de la estrategia es que separa los detalles de la implementación en su propia clase y, por lo tanto, permite cambiar las estrategias fácilmente durante el tiempo de ejecución si es necesario. Esto también fomenta la idea de una responsabilidad por clase solamente.

 

As motivated by @Vogel612 I post my suggestion as an answer. An answer should probably contain a bit more than just a link to the strategy pattern therefore I'll adept the pattern to your presented code.

Instead of keeping the logic in the Filter class, this class is refactored to an interface which provides a simple apply() method.

public interface Filter {     Conversation apply(Conversation conversation, String argumentValue); } 

If you need certain base logic you could also use an abstract base class instead of an interface of course.

A concrete implementation of a filter could now look like this:

public class UserFilter implements Filter {     @Override     public Conversation apply(Conversation conversation, String specificUser) {         List<Message> messageList = new ArrayList<>();         //Filter by used id         for (Message message : conversation.messages) {             if (message.senderId.equals(specificUser)) {                 messageList.add(message);                 Conversation filteredConversation =                      new Conversation(conversation.name, messageList);                 conversation = filteredConversation;             }         }         return conversation;     } } 

This allows you to refactor the filter method to something like this:

private void filter(Filter filter, Conversation conversation, String outputFilePath) throws Exception {     if (filter != null) {         String argumentValue = filter.argumentValue;         conv = filter.apply(conversation, argumentValue);     }     this.writeConversation(conversation, outputFilePath); } 

This also makes the filterType property redundant which thus can be removed. However, instead of keeping a single Filter class around, you need to create a concrete instance of a filter and pass it around. As you did not include the code where you set the filterType, I can't give more code on where the concrete object should be instantiated.

One advantage of the strategy pattern is, that it separates the implementation details into its own class and therefore allows switching strategies easily during runtime if needed. This also fosters the idea of one responsibility per class only.

 
 

Relacionados problema

8  Simple GCD Utility en Java  ( Simple gcd utility in java ) 
i anteriormente discutido El rendimiento se refiere a diferentes algoritmos GCD. Escribí una simple clase de Java que implementa el algoritmo binario GCD. E...

1  Compruebe si dos cadenas son permutación entre sí  ( Check if two strings are permutation of each other ) 
private String sort(String word) { char[] content = word.toCharArray(); Arrays.sort(content); return new String(content); } private boolea...

2  Eliminación de un nodo en un árbol de búsqueda binario  ( Deletion of a node in a binary search tree ) 
Estoy buscando ver si mi implementación del método de eliminación / eliminación en un árbol de búsqueda binario es suficiente, legible y se ejecuta en un tiem...

17  Implementación vectorial (física)  ( Vector physics implementation ) 
Recientemente comencé a aprender Java, y decidí implementar un sistema de vectores básico para otro sistema de partículas que estaba construyendo. join()9 ...

34  Clon a todo color del juego de la vida de Conway, con una GUI decente  ( Full color clone of conways game of life with a decent gui ) 
Escribí esto para aprender Javafx, y como excusa para volver a hacer el juego de la vida. Esta es la gui más compleja que he escrito, así que me gustaría come...

5  Memoria / Performance of Merge Sort Code  ( Memory performance of merge sort code ) 
Escribí un código de tipo de combinación para un poco de bocadillo nocturno. Lo he puesto trabajando, pero solo estaba mirando a aprender si me faltaba algo e...

5  Encuentre el próximo número Prime - Control de flujo de los bucles anidados 'para `  ( Find the next prime number flow control of nested for loops ) 
Este código funciona perfectamente, pero me molesta. Tener un bucle etiquetado y anidado Bucle, con un Enumerable<T>.Empty()0 Declaración, y un 9988777665...

6  Encontrar el siguiente palíndromo de una cadena de números  ( Finding the next palindrome of a number string ) 
Aquí está el problema: Un entero positivo se llama palíndromo si su representación en el El sistema decimal es el mismo cuando se lee de izquierda a dere...

2  Solucionador de rompecabezas de rascacielos en Java [cerrado]  ( Skyscraper puzzle solver in java ) 
cerrado. Esta pregunta es off-topic . Actualmente no está aceptando respuestas. ¿Quieres ...

2  Fusionar la implementación de Sort Java  ( Merge sort java implementation ) 
¿Puede alguien revisar mi implementación de tipo de fusión en Java? ¿Es bueno / malo y puede mejorarse más? public class MyMergeSort { private int [] d...




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