Encontrar y mostrar la posición de una palabra en una oración -- vb.net camp codereview Relacionados El problema

Find and display the position of a word in a sentence


1
vote

problema

Español

¿Cómo puedo simplificar este código:

  Number7  
Original en ingles

How can I simplify this code:

Public Class Form1 Private Sub BtnPositions_Click(sender As Object, e As EventArgs) Handles BtnPositions.Click     ' 'txtSentence.Text = "The cat sat on the mat" ' Temp for testing      If TxtAddSentence.Text = "" Then         MsgBox("The text you are searching is empty, so there are no matches." & vbCrLf & "Please enter some text and retry.")     End If      Dim WordArray() = {""}     Dim strSearchWord As String      BuildWordArray(TxtAddSentence.Text, WordArray)     strSearchWord = TxtAddWord.Text     FindPositionOfWords(strSearchWord, WordArray)      If TxtAddSentence.Text = "" Then         MsgBox("The sentence box is empty, so there are no matches.")     End If  End Sub Private Sub BuildWordArray(ByVal strText, ByRef strArray)     Dim strTextArray1() = Split(strText, " ")     strArray = strTextArray1 End Sub  Private Sub FindPositionOfWords(ByVal strSearchWord, ByVal strWordList)      Dim intIndex As Integer = 1     Dim strPosn As String = ""     For Each strWord As String In strWordList         If UCase(strWord) = UCase(strSearchWord) Then             If strPosn = "" Then                 strPosn = strPosn & Str(intIndex)             Else                 strPosn = strPosn & "," & Str(intIndex)             End If         End If         intIndex = intIndex + 1     Next     If strPosn = "" Then         MsgBox("The Search Word Here Box Is Empty", MsgBoxStyle.Exclamation)     Else         MsgBox("The word '" & strSearchWord & "' was found at positions" & strPosn)     End If End Sub End Class 
  

Lista de respuestas

2
 
vote

Tenga en cuenta que no soy tan grande en vb.net

Variables - ¡Defina! No has definido

     TxtAddWord    TxtAddSentence (though I see txtSentence commented out)   

¿Se definen en el diseño del formulario?


Pruebas de algo para ser "" Por lo general, tiene una mejor manera. Creo que algo así -

  If String.IsNullOrEmpty(txtaddsentence) Then   

Hablando de ese if - lo tiene dos veces, ¿por qué?

  If TxtAddSentence.Text = "" Then     MsgBox("The sentence box is empty, so there are no matches.") End If   

No puedo ver TxtAddSentence Cambio después de la primera verificación, ya que no hay ninguna función para devolver una cadena.


Hablando de cuerdas, ¿por qué siempre estás usando

  TxtAddSentence.Text   

cuando puedes simplemente

      Dim TxtAddSentence = "The cat sat on the mat" ' Temp for testing     Dim TxtAddWord = "sat"   

que son string Tipo.


NOMBACIÓN: use nombres de variables para describir lo que está sucediendo, y evite eso notación húngara .

  Variable -> can be -> TxtAddSentence      sentenceAddition TxtAddWord          wordToAdd strSearchWord       searchWord strText             why? it's already sentenceAddition strArray            why? it's already wordArray strWordList         what is this? a List Type? An Array Type? A String?        intIndex            index strPosn             searchPosition strWord             listedWord   

Además, no está dando un tipo a sus variables:

  ""0  

puede ser, por lo que puedo decir

  ""1  

sería la mejor práctica dar un tipo a las variables en ""2 - Pero no estoy completamente seguro de qué tipo deberían ser.

aquí -

  ""3  

Si esta es una posición, ¿por qué no es un entero? Esperaría un número entero, no una cadena.

 

please note I am not that great at vb.net

Variables - Define them! You haven't defined

   TxtAddWord    TxtAddSentence (though I see txtSentence commented out) 

are they defined on the form design?


Testing for something to be "" usually has a better way. I think something like this -

If String.IsNullOrEmpty(txtaddsentence) Then 

Speaking of that if - you have it twice - why?

If TxtAddSentence.Text = "" Then     MsgBox("The sentence box is empty, so there are no matches.") End If 

I can't see TxtAddSentence changing after the first check as there aren't any functions to return a string.


Speaking of strings, why are you always using

TxtAddSentence.Text 

When you can just

    Dim TxtAddSentence = "The cat sat on the mat" ' Temp for testing     Dim TxtAddWord = "sat" 

Which are string type.


Naming - use variable names to describe what is happening, and avoid that hungarian notation.

Variable -> can be -> TxtAddSentence      sentenceAddition TxtAddWord          wordToAdd strSearchWord       searchWord strText             why? it's already sentenceAddition strArray            why? it's already wordArray strWordList         what is this? a List Type? An Array Type? A String?        intIndex            index strPosn             searchPosition strWord             listedWord 

Additionally, you're not giving a type to your variables:

ByVal strText, ByRef strArray) ByVal strSearchWord, ByVal strWordList 

Can be, as far as I can tell

Private Sub FindPositionOfWords(ByVal searchWord As String, ByVal wordArray() As String) Private Sub BuildWordArray(ByVal sentenceAddition, ByRef wordArray) 

It would be best practice to give a type to the variables in BuildWordArray - but I'm not entirely sure what type they should be.

Here -

Dim strPosn As String = "" 

If this is position, why isn't it an integer? I would expect an integer, not a string.

 
 
1
 
vote

Hay algunas cosas que cambiaría. Algunos están relacionados con el rendimiento (aunque en su caso no harán ninguna diferencia real, aunque es una buena práctica) y otros simplemente reducen la cantidad de código. Otro cambio debe hacer que su código sea menos probable que no encuentre palabras. Ok aquí es.


Esta línea hace cumplir las prácticas de programación adecuadas en el caso de las conversiones de tipo y sever otras cosas.

  Option Strict On   

Esta línea impone la declaración de tipo en lugar de que el compilador los infiera de otro código. Obtendrá menos problemas en los futuros programas si siempre lo quita.

  Option Infer Off   

Esta línea importa el código que le permite usar herramientas de manejo de texto poderosos (aunque no muy legibles). Ver más tarde.

  Imports System.Text.RegularExpressions  Public Class Form1     Private Sub BtnPositions_Click(sender As Object, e As EventArgs) Handles BtnPositions.Click   

Las siguientes dos líneas se han trasladado a la parte superior de la clase: a menudo es una buena práctica para tener sus variables en la parte superior de la definición para que sean más fáciles de encontrar. Además, este es el bit que utiliza REGEX como se mencionó anteriormente en las importaciones en la parte superior del código. Esta línea elimina todo lo que no es un espacio o un carácter alfabético. Por ejemplo, si su oración es "El gato es grande, gordo y peludo", si intenta buscar "Big", usando su código original, no obtendrá un resultado porque la palabra almacenada en strWordList sería "grande", no "grande"

          Dim WordArray() As String = {""}         Dim sentenceToSearch As String = Regex.Replace(TxtAddSentence.Text, "[^a-zA-Z ]", "").ToLower   

Compruebe si ambos cuadros de texto tienen texto en. Si no lo hacen, no hay necesidad de ejecutar el resto del código, por lo tanto, el uso de Exit Sub .

          If TxtAddSentence.Text = "" Then             MsgBox("The sentence box is empty, so there are no matches.")             Exit Sub         ElseIf TxtAddWord.Text = "" Then             MsgBox("The search box is empty, so there are no matches." & vbCrLf & "Please enter some text and retry.")             Exit Sub         End If   

`BuildwordArray es mejor como una función que como un Sub para la legibilidad y más tarde en programas más grandes, la capacidad de mantenimiento.

          WordArray = BuildWordArray(TxtAddSentence.Text)   

En esta situación, sin embargo, el Sub no cambia ninguna variable / objetos fuera de sí misma, por lo que está bien dejarlo como lo es.

          FindPositionOfWords(TxtAddWord.Text, WordArray)     End Sub   

Esta función solo necesita la línea como strTextArray1 solo una variable local y realmente no lo necesita en este caso, ya que puede devolver directamente los resultados del Option Infer Off 0 Función. Además, este es el punto en el que la cadena puede convertirse con seguridad en minúsculas. En lugar de tener el programa haciendo varias conversiones en el bucle de búsqueda, solo se realiza una vez aquí.

  Option Infer Off 1  

ver más arriba también para mi explicación de Option Infer Off 2

  Option Infer Off 3  

En lugar de usar un bucle 99887766655443314 998877766555443315 bucle y use su índice. Así que aquí no hay definición de 'intindex1 hasta el bucle aquí.

  Option Infer Off 6  

Vea mi explicación anterior sobre cómo convertir las cadenas en minúsculas en minúsculas antes de En el Declaración Option Infer Off 7 Ahora solo tiene la otra en lugar de dos sentencias Option Infer Off 8 . Esto nuevamente, en programas más grandes mejorará el rendimiento. En este código, es más sencillo simplemente seguir agregando la variable 99887766655443319 Imports System.Text.RegularExpressions Public Class Form1 Private Sub BtnPositions_Click(sender As Object, e As EventArgs) Handles BtnPositions.Click 0 podría ser confuso con una matriz o alguna otra colección, por lo que es mejor usar Imports System.Text.RegularExpressions Public Class Form1 Private Sub BtnPositions_Click(sender As Object, e As EventArgs) Handles BtnPositions.Click 1 . No hay necesidad en este caso para tener el control adicional con respecto a la verificación. Para agregar comas, simplemente recorte la última coma después de que se haya completado el bucle.

  Imports System.Text.RegularExpressions  Public Class Form1     Private Sub BtnPositions_Click(sender As Object, e As EventArgs) Handles BtnPositions.Click 2  

El cuadro de mensaje en su versión original del código mostró un mensaje incorrecto. Anteriormente, en el nuevo código, ya se ha revisado el cuadro de texto de búsqueda para asegurarse de que no estaba vacío, por lo que el mensaje debería ser simplemente que no se han encontrado resultados. Decirle al usuario que la caja está vacía los confundiría.

  Imports System.Text.RegularExpressions  Public Class Form1     Private Sub BtnPositions_Click(sender As Object, e As EventArgs) Handles BtnPositions.Click 3  

Eso es todo. Para que sea más fácil para usted utilizar el código actualizado .. Aquí está en un bloque

  Imports System.Text.RegularExpressions  Public Class Form1     Private Sub BtnPositions_Click(sender As Object, e As EventArgs) Handles BtnPositions.Click 4  
 

There are a few things I would change. Some are performance related(though in your case they wont make any real difference, its good practice though) and others simply reduce the amount of code. Another change should make your code less likely to not find words. OK Here it is.


This line enforces proper programming practices in the case of type conversions and sever other things.

Option Strict On 

This line enforces declaration of type rather than having the compiler infer them from other code. You'll get less problems in future programs if you always have this off.

Option Infer Off 

This line imports code that allows you to use powerful(though not very readable) text handling tools. See later on.

Imports System.Text.RegularExpressions  Public Class Form1     Private Sub BtnPositions_Click(sender As Object, e As EventArgs) Handles BtnPositions.Click 

the next two lines have been moved to the top of the Class - it's often good practice to have your variables at the top of the definition so they are easier to find. Also, this is the bit that uses Regex as mentioned above in the imports at the top of the code. This line removes everything that is not a space or an alphabetic character. For example, if your sentence is "The cat is big, fat and hairy", if you try searching for "big", using your original code, you won't get a result because the word stored in strWordList would be "big," not "big"

        Dim WordArray() As String = {""}         Dim sentenceToSearch As String = Regex.Replace(TxtAddSentence.Text, "[^a-zA-Z ]", "").ToLower 

Check if both of your text boxes have text in. If they don't then there is no need to execute the rest of the code, hence the use of Exit Sub.

        If TxtAddSentence.Text = "" Then             MsgBox("The sentence box is empty, so there are no matches.")             Exit Sub         ElseIf TxtAddWord.Text = "" Then             MsgBox("The search box is empty, so there are no matches." & vbCrLf & "Please enter some text and retry.")             Exit Sub         End If 

`BuildWordArray is better as a Function than as a Sub for readability and later on in larger programs, maintainability.

        WordArray = BuildWordArray(TxtAddSentence.Text) 

In this situation however, the sub doesn't change any variables/objects outside of itself, so it is fine to leave it as it is.

        FindPositionOfWords(TxtAddWord.Text, WordArray)     End Sub 

This function only needs the one line as strTextArray1 only a local variable and you don't really need it in this case as you can directly return the results of the Split function. Also, this is the point where the string can safely be converted to lower case. Rather than have the program doing several conversions in the search loop, it is only done once here.

    Private Function BuildWordArray(ByVal strText As String) As String()         Return Split(strText.ToLower, " ")     End Function       Private Sub FindPositionOfWords(ByVal strSearchWord As String, ByVal strWordList() As String) 

See above also for my explanation of Regex

        strSearchWord = Regex.Replace(strSearchWord, "[^a-zA-Z ]", "").ToLower         Dim strPosn As String = "" 

Rather than use a For Each..Next loop and still use a variable as an index, just just a For..Next loop and use it's index. So here there is no definition of `intIndex1 until the loop here.

        For intIndex As Integer = 1 To strWordList.Count 

See my explanation above about converting strings to lower case earlier on In the If statement you now only have the one instead of two If statements. This will again, in larger programs improve performance. In this code it's simpler just to keep adding the intIndex variable. The method Strcould be confusre with an array or some other collection, so it is better to use intIndex.ToString.There is no need in this case to have the extra check with regard to the adding of commas, Just trim off the last comma after the loop has completed.

            If strWordList(intIndex) = strSearchWord Then                 strPosn = strPosn & intIndex.ToString & ","             End If         Next         strPosn.TrimEnd(","c) 

The message box in you original version of the code showed an incorrect message. Earlier on in the new code, the search textbox has already been checked to make sure that it wasn't empty, so the Message should simply be that no results have been found. Telling the user that the box is empty would confuse them.

        If strPosn = "" Then             MsgBox("No results found", MsgBoxStyle.Exclamation)         Else             MsgBox("The word '" & strSearchWord & "' was found at positions" & strPosn)         End If     End Sub End Class 

That's it. To make it easier for you to use the updated code .. here it is in one block

Option Strict On Option Infer Off  Imports System.Text.RegularExpressions  Public Class Form1     Private Sub BtnPositions_Click(sender As Object, e As EventArgs) Handles BtnPositions.Click         Dim WordArray() As String = {""}         Dim sentenceToSearch As String = Regex.Replace(TxtAddSentence.Text, "[^a-zA-Z ]", "").ToLower         If TxtAddSentence.Text = "" Then             MsgBox("The sentence box is empty, so there are no matches.")             Exit Sub         ElseIf TxtAddWord.Text = "" Then             MsgBox("The search box is empty, so there are no matches." & vbCrLf & "Please enter some text and retry.")             Exit Sub         End If         WordArray = BuildWordArray(TxtAddSentence.Text)         FindPositionOfWords(TxtAddWord.Text, WordArray)     End Sub      Private Function BuildWordArray(ByVal strText As String) As String()         Return Split(strText.ToLower, " ")     End Function      Private Sub FindPositionOfWords(ByVal strSearchWord As String, ByVal strWordList() As String)         strSearchWord = Regex.Replace(strSearchWord, "[^a-zA-Z ]", "").ToLower         Dim strPosn As String = ""         For intIndex As Integer = 1 To strWordList.Count             If strWordList(intIndex) = strSearchWord Then                 strPosn = strPosn & intIndex.ToString & ","             End If         Next         strPosn.TrimEnd(","c)         If strPosn = "" Then             MsgBox("No results found", MsgBoxStyle.Exclamation)         Else             MsgBox("The word '" & strSearchWord & "' was found at positions" & strPosn)         End If     End Sub End Class 
 
 

Relacionados problema

3  Manejar múltiples menú Haga clic en eventos en un submarino  ( Handle multiple menu click events in one sub ) 
Tengo múltiples menús que cambiarán los idiomas en un Sub que manejarán múltiples eventos de los menús, se está trabajando para mí en este momento, me pregunt...

0  VB.NET - Manejo de errores en clase genérica para PDF Fusion  ( Vb net error handling in generic class for pdf merge ) 
Como parte de un proyecto en el que estoy trabajando, ha surgido un nuevo requisito para poder generar múltiples archivos PDF y luego fusionarlos antes de que...

2  Resolución de la pantalla  ( Screen resolution ) 
Estoy trabajando con una aplicación que tendrá sub formularios. La forma primaria se abre por encima de la bandeja del sistema, y ​​en el ratón, las ventanas ...

2  Implementar el tipo de interfaz en la implementación de otra interfaz sin demasiado casting  ( Implement interface type in implementation of another interface without too much ) 
Tengo un grupo de pruebas que deben ejecutarse de manera similar (pero diferente) y tienen configuraciones, resultados, etc. que también son similares pero di...

3  ¿Cómo reformatear este código para que no utilice 'SALIR PRUY' y 'SALIR POR'?  ( How to reformat this code so i dont use exit try and exit for ) 
Utilizo Sonarqube para ayudarme a intentar convertirme en un mejor programador para programar junto con los estándares de "mejores prácticas". Hay reglas co...

4  Simplify ASP.NET StreamEtriter para APPENDTEXT a archivos  ( Simplify asp net streamwriter for appendtext to file ) 
¿Hay alguna manera de simplificar este código, por lo que no hay tantos 99887766555443388 var o = {}; o.window = o; $.isWindow(o); //returns `true`, even th...

5  Model-View-Presenter  ( Model view presenter ) 
Trabajo en una aplicación .NET que sigue de forma muy apósito para un tipo de arquitectura de N-Tier (los objetos de negocios (no lógica) y el acceso a los da...

1  Asignó un recurso del sistema a una imagen basada en variable  ( Assigned a system resource to an image based on variable ) 
Tengo una aplicación de mapeo y el marcador que estoy usando se basa en el representante de campo asignado a esa visita. Mi código es solo un caso selecto bas...

3  ¿Es esta la forma correcta de exponer una configuración para una biblioteca de clase VB.NET?  ( Is this the correct way to expose a setting for a vb net class library ) 
Estoy definido en la pestaña Configuración de una biblioteca de clase VB.NET, algunas configuraciones de alojamiento de usuario. Quiero exponer estas configur...

5  WebMethod más lento en la ejecución que la aplicación de Windows del mismo código  ( Webmethod slower on execution than windows application of the same code ) 
He desarrollado una aplicación que interactúa con IBM ClearQuest. El problema es que cuando ejecuto todo localmente, como, ejecute el local de WebService y lu...




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