Comprobación y conversión [[HH:] mm:] Formato de entrada SS -- datetime campo con bash camp codereview Relacionados El problema

Checking and converting [[HH:]MM:]SS input format


6
vote

problema

Español

Me gustaría usar la siguiente rutina en mi script de bash de envío de trabajo que espera la Tiempo Fondo permitido en el formato private static int[] Shuffle(int n) { var a = new int[n]; var random = new Random(); for (int i = 0; i < a.Length; i++) { var j = random.Next(0, i); Swap(a, i, j); } return a; } private static void Swap(int[] a, int i, int j) { var temp = i; a[i] = a[j]; a[j] = temp; } 3 . (Soportes que indican opcionalidad). Para facilitar la legibilidad, quiero convertir cualquier valor de entrada en el formato fijo private static int[] Shuffle(int n) { var a = new int[n]; var random = new Random(); for (int i = 0; i < a.Length; i++) { var j = random.Next(0, i); Swap(a, i, j); } return a; } private static void Swap(int[] a, int i, int j) { var temp = i; a[i] = a[j]; a[j] = temp; } 4 donde se abre el valor de las horas.
Creo que funciona bien, pero me preocupa un poco si he pasado por alto los posibles puntos donde un usuario podría cometer un error. Esto podría llevar a presentar un grupo de empleo no válido y muchos trabajos de computación que no están comenzando.
También me pregunto si mi código es o no óptimo.

  private static int[] Shuffle(int n) {     var a = new int[n];     var random = new Random();     for (int i = 0; i < a.Length; i++) {         var j = random.Next(0, i);         Swap(a, i, j);     }     return a; }  private static void Swap(int[] a, int i, int j) {     var temp = i;     a[i] = a[j];     a[j] = temp; } 5  

Utilizo las dos primeras funciones bastante en mi script, así que las usé para la función de destino también.

Original en ingles

I would like to use the following routine in my job submission bash script which expects the allowed walltime in the format [[HH:]MM:]SS. (Brackets indicating optionality.) For easier readability I want to convert any input value into the fixed format HH:MM:SS where the value for hours is open ended.
I think it works okay, but I am a bit worried if I have overlooked possible points where a user could make a mistake. This could lead to submitting an invalid jobscript and a lot of computation jobs not starting.
I also ask myself whether or not my code is optimal.

#!/bin/bash  # # If there is an unrecoverable error: display a message and exit. #  printExit () {     case $1 in         [iI]) echo INFO: "$2" ;;         [wW]) echo WARNING: "$2" ;;         [eE]) echo ERROR: "$2" ; exit 1 ;;            *) echo "$1" ;;     esac }  # # Test if a given value is an integer #  isnumeric () {     if ! expr "$1" : '[[:digit:]]*$' > /dev/null; then         printExit E "Value for $2 ($1) is no integer."     fi }  # # Test if a given value is a proper duration #  istime () {     local tempTime=$1     # Split time in HH:MM:SS     theTempSeconds=${tempTime##*:}     isnumeric "$theTempSeconds" "seconds"     if [[ ! "$tempTime" == "${tempTime%:*}" ]]; then       tempTime="${tempTime%:*}"       theTempMinutes=${tempTime##*:}       isnumeric "$theTempMinutes" "minutes"     fi     if [[ ! "$tempTime" == "${tempTime%:*}" ]]; then       tempTime="${tempTime%:*}"       theTempHours=${tempTime##*:}       isnumeric "$theTempHours" "hours"     fi     if [[ ! "$tempTime" == "${tempTime%:*}" ]]; then       printExit E "Unrecognised format."     fi      theSeconds=`expr $theTempSeconds % 60`     theTempMinutes=`expr $theTempMinutes + $theTempSeconds / 60`     theMinutes=`expr $theTempMinutes % 60`     theHours=`expr $theTempHours + $theTempMinutes / 60`      printf -v theWalltime "%d:%02d:%02d" $theHours $theMinutes $theSeconds }  theWalltime="$1"  echo "$theWalltime"  istime $theWalltime  echo "$theWalltime"  exit 0 

I use the first two functions quite a lot in my script so I used them for the target function also.

     

Lista de respuestas

1
 
vote
vote
La mejor respuesta
 

Aquí están mis sugerencias:

  • El printExit() imprime un mensaje de error, pero no sale. Sería mejor en realidad exit o cambie el nombre de la función.
  • Su regex en isnumeric() tiene dos problemas menores. Poner un ^ Al principio, dejará en claro que el regex coincida con toda la cadena. Si bien esto está implícito con expr no se ajusta a la forma en que la mayoría de las implementaciones funcionan. Otro problema es que el quantificador 9988776655544335 significa 0 o más para que pueda coincidir 0 o nada de lo que está buscando. Por lo tanto, una cadena vacía coincidiría con lo que tiene, ya que serían cero dígitos seguidos por el final de la cadena. No pude reproducir este modo de falla, pero recomendaría esta línea en lugar de qué funciona para mis pruebas:

      if ! expr "$1" : '^[[:digit:]]+$' > /dev/null; then   
  • Sería bueno explicar lo que está tratando de hacer con esas manipulaciones de cadenas con algunos comentarios
  • MÁS DE SUS VARIABLES PODRÍA SER local IZED Para que no se escapen de la función en la que se usan.
  • Uso de the En los nombres de las variables para evitar contradictorios con los nombres genéricos funciona, pero si puede encontrar nombres más descriptivos, sería más fácil seguir. A lo largo de las mismas líneas, todas las variables son temp orary, por lo que no ayuda a explicar por qué existe la variable. Tal vez exit0 o incluso exit1 .
  • Todas las matemáticas son matemáticas enteras. No obtendrá ningún decimal con exit2 o 99887766555443313 Matemáticas. https://stackoverflow.com/questions/12722095/HOW-CAN-i-ET- Float-Division explica que puede trabajar con exit4 .
  • optimización usando exit5 está bien, pero llama un programa externo. Uso de exit6 's Matemáticas integradas evitaría deslear un nuevo proceso y reduciría mucho la hora del guión. ((just_minutes = just_minutes + trunc_seconds / 60))
 

Here are my suggestions:

  • The printExit() prints an error message, but does not exit. It would be better to actually exit or rename the function.
  • Your regex in isnumeric() has two minor issues. Putting a ^ at the beginning will make it clear that the regex matches the whole string. While this is implicit with expr it does not fit with how most implementations work. Another issue is that the * quantifier means 0 or more so it might match 0 or none of the what you're looking for. So an empty string would match what you have since it would be zero digits followed by the end of the string. I couldn't reproduce this failure mode, but I'd recommend this line instead which works for my tests:

    if ! expr "$1" : '^[[:digit:]]\+$' > /dev/null; then 
  • It would be nice to explain what you're trying to do with those string manipulations with a few comments
  • More of your variables could be localized so they don't leak out of the function they are used in.
  • Using the in variable names to avoid conflicting with generic names works, but if you can find more descriptive names it would be easier to follow. Along the same lines all variables are temporary so that doesn't help explain why the variable exists. Maybe checkTime or even check_time.
  • All of the math is integer math. You won't get any decimals with bash or expr math. https://stackoverflow.com/questions/12722095/how-can-i-get-float-division explains that you can work around with bc.
  • Optimization Using expr is fine, but it calls an external program. Using bash's builtin math would avoid spawning a new process and would cut out a lot of the script's time. ((just_minutes = just_minutes + trunc_seconds/60))
 
 
 
 
1
 
vote

He actualizado el código con algunas de las sugerencias de las polluelas.

  • Mientras que el exit7 se pretende inicialmente para salir realmente en cualquier escenario dado, se transmutó a lo largo de algunas iteraciones en otros scripts en una rutina de información algo más avanzada. Ahora he cambiado el nombre para reflejar eso y eliminará lentamente esto en todos mis scripts en el futuro
  • La expresión regular ahora coincide con toda la expresión y no puede ser cero
  • Los nombres de las variables reflejan ahora lo que realmente están haciendo y se establecieron como los locales siempre que sea posible para evitar fugas (que podrían ocurrir si el usuario aplica la opción de duración dos veces)
  • La transformación ahora se realiza mediante el bash incorporado en exit8 , es de matemáticas enteras, al igual que la intención
  • probablemente La mayor mejora en la optimización del código es anidando las declaraciones si está comprobando la declaración de entrada. Si la duración se proporcionó en exit9 , ahora solo está comprobando una sola vez si isnumeric()0 y lo mismo se aplica a minutos isnumeric()1 (2) y horas isnumeric()2 (4, ya que necesita comprobación si hay sobras).

Aquí está el código completo (cuando se realiza todo el script, lo pondré a disposición a través de un sitio web de terceros):

  isnumeric()3  
 

I have updated the code with some of the suggestions from chicks answer.

  • while the prinExit() routine was initially intended to actually exit in any scenario given, it transmuted over the course of a few iterations in other scripts into a somewhat more advanced info routine. I have now changed the name to reflect that and will slowly roll this out in all of my scripts in the future
  • the regular expression now matches the whole expression and cannot be zero
  • the variable names reflect now what they are actually doing and they were set as local ones wherever possible to prevent leaking (which could happen if the user applies the duration option twice)
  • the transformation is now done by the bash built in $(()), it is integer maths as is the intention
  • probably the biggest improvement in optimising the code is nesting the if statements while checking the input statement. If the duration was given in SS it is now only checking once if [[ ! "$foo" == "${foo%:*}" ]] and the same applies to minutes MM:SS (2) and hours HH:MM:SS (4, as it needs checking if there are leftovers).

Here is the complete code (when the whole script is done I'll make it available through a third party website):

#!/bin/bash   # # Print information and warnings. # If there is an unrecoverable error: display a message and exit. #  printInfo () {     case $1 in         [iI]) echo INFO: "$2" ;;         [wW]) echo WARNING: "$2" ;;         [eE]) echo ERROR: "$2" ; exit 1 ;;            *) echo "$1" ;;     esac }  # # Test if a given value is an integer #  isnumeric () {     if ! expr "$1" : '^[[:digit:]]\+$' > /dev/null; then         printInfo E "Value for $2 ($1) is no integer."     fi }  # # Test if a given value is a proper duration # Code reviewed: http://codereview.stackexchange.com/q/114144/92423 #  isduration () {     local checkDuration=$1     # Split time in HH:MM:SS     # Strips away anything up to and including the rightmost colon     # strips nothing if no colon present     # and tests if the value is numeric     # this is assigned to seconds     local truncDuration_Seconds=${checkDuration##*:}     isnumeric "$truncDuration_Seconds" "seconds"     # If successful value is stored for later assembly     #     # Check if the value is given in seconds      # "${checkDuration%:*}" strips shortest match ":*" from back     # If no colon is present, the strings are identical     if [[ ! "$checkDuration" == "${checkDuration%:*}" ]]; then       # Strip seconds and colon        checkDuration="${checkDuration%:*}"       # Strips away anything up to and including the rightmost colon       # this is assigned as minutes       # and tests if the value is numeric       local truncDuration_Minutes=${checkDuration##*:}       isnumeric "$truncDuration_Minutes" "minutes"       # If successful value is stored for later assembly       #       # Check if value was given as MM:SS same procedure as above       if [[ ! "$checkDuration" == "${checkDuration%:*}" ]]; then         #Strip minutes and colon         checkDuration="${checkDuration%:*}"         # # Strips away anything up to and including the rightmost colon         # this is assigned as hours         # and tests if the value is numeric         local truncDuration_Hours=${checkDuration##*:}         isnumeric "$truncDuration_Hours" "hours"         # Check if value was given as HH:MM:SS if not, then exit         if [[ ! "$checkDuration" == "${checkDuration%:*}" ]]; then           printInfo E "Unrecognised format."         fi       fi     fi      # Modify the duration to have the format HH:MM:SS      # disregarding the format of the user input     local finalDuration_Seconds=$((truncDuration_Seconds % 60))     # Add any minutes that had overflow to the minutes given as input     truncDuration_Minutes=$((truncDuration_Minutes + truncDuration_Seconds / 60))     # save what cannot overflow as hours     local finalDuration_Minutes=$((truncDuration_Minutes % 60))     # add any minutes the overflew to hours     local finalDuration_Hours=$((truncDuration_Hours + truncDuration_Minutes / 60))      # Format string and saveon variable     printf -v requestedWalltime "%d:%02d:%02d" $finalDuration_Hours $finalDuration_Minutes \                                          $finalDuration_Seconds }  requestedWalltime="$1"  echo "$requestedWalltime"  isduration $requestedWalltime  echo "$requestedWalltime"  exit 0 
 
 
1
 
vote

Una alternativa a printExit

Como @chicks ya señaló, el 9988776655544331 no es un buen nombre, ya que no siempre sale, y tampoco es obvio lo que hará un método de "salida de impresión". Un punto relacionado, encuentro los parámetros I-W-E un poco críptico. Recomiendo una alternativa que utilice para este propósito:

  msg() {     echo $* }  warn() {     msg WARNING: $* }  fatal() {     msg FATAL: $*     exit 1 }   

es decir:

  • warn "some message" es más natural que printExit W "some message"
  • warn some message también funciona (sin comillas dobles)
  • Si quiero buscar todas las ocurrencias de advertencias en el Código, es un poco más fácil escribir "Warn" que "PrintExit [WW]". (En realidad, dado que utilizo vim para editar scripts, puedo presionar * en la palabra "advertir" para encontrar todas las apariciones, para que se vuelva literalmente una sola pulsación.)

Más sobre nombrar cosas

Funciones prefijadas con la palabra "es" implica una operación booleana, devolviendo verdadero o falso. En caso de bash, el éxito de regreso o el fracaso. A diferencia de eso:

  • El isnumeric sale del programa si el param no es numérico
  • El istime puede llevar a salir de las llamadas a printExit0 , o si todo está bien, imprime el tiempo

Sería mejor cambiarlos. Por ejemplo, printExit1 sería un mejor nombre, ya que el prefijo "Validar" se usa comúnmente para las operaciones que aumentan la excepción si las pruebas requeridas fallan.

Por último, es bueno enfatizar visualmente los límites de las palabras de los términos distintos en los nombres de las funciones, por ejemplo, 99887766555443312 o printExit3 .

Mejora printExit4

Sugerir que hacer printExit5 una función booleana real e introducir una nueva función printExit6 para salir en caso de no numérico:

  printExit7  

Mejora printExit8

No se recomienda usar printExit9 hoy en día. Genera un proceso adicional, no es portátil, y las mejores alternativas generalmente están disponibles.

En este ejemplo, puede reescribir usando msg() { echo $* } warn() { msg WARNING: $* } fatal() { msg FATAL: $* exit 1 } 0 como este:

  msg() {     echo $* }  warn() {     msg WARNING: $* }  fatal() {     msg FATAL: $*     exit 1 } 1  

Nitpicks

El 99887766555443322 es innecesario.

 

An alternative to printExit

As @chicks already pointed out, the printExit is not a good name, as it doesn't always exit, and it's also not obvious what a "print exit" method will do. A related point, I find the i-w-e parameters a bit cryptic. I recommend an alternative I use for this purpose:

msg() {     echo $* }  warn() {     msg WARNING: $* }  fatal() {     msg FATAL: $*     exit 1 } 

That is:

  • warn "some message" is more natural than printExit W "some message"
  • warn some message also works (without double quotes)
  • If I want to search for all occurrences of warnings in the code, it's a bit easier to type "warn" than "printExit [wW]". (Actually, since I use vim to edit scripts, I can just press * on the word "warn" to find all occurrences, so that becomes literally a single keystroke.)

More on naming things

Functions prefixed with the word "is" imply some boolean operation, returning true or false. In case of Bash, returning success or failure. As opposed to that:

  • the isnumeric function exits the program if the param is not numeric
  • the istime function may lead to exiting in calls to isnumeric, or else if everything was fine it prints time

It would be better to rename them. For example, validate_numeric would be a better name, as the "validate" prefix is commonly used for operations that raise exception if the required tests fail.

Lastly, it's good to emphasize visually the word boundaries of the distinct terms in function names, for example is_numeric or isNumeric.

Improving isnumeric

I suggest making is_numeric a real boolean function, and introduce a new function validate_numeric to exit in case of not numeric:

is_numeric() {     expr "$1" : '[[:digit:]]*$' > /dev/null }  validate_numeric() {     if ! is_numeric "$1"; then         fatal "Value for $2 ($1) is no integer."     fi } 

Improving is_numeric

It's not recommended to use expr anymore today. It spawns an extra process, it's not portable, and better alternatives are usually available.

In this example, you can rewrite using [[ ... =~ ... ]] like this:

is_numeric() {     [[ $1 =~ ^[[:digit:]]+$ ]] } 

Nitpicks

The final exit 0 is unnecessary.

 
 
   
   

Relacionados problema

7  Contando segundos hasta el cierre automático  ( Counting seconds until auto log off ) 
Acabo de crear un pequeño script para adolescentes para contar los segundos hasta que me desconecte automáticamente. Ya resolví mi problema con mis ajustes de...

3  Funciones de Bash para ejecutar / administrar MySQL, Redis, Sidekiq, Rails Server  ( Bash functions to run manage mysql redis sidekiq rails server ) 
Estoy trabajando con una aplicación de Rails que requiere algunas capas de infraestructura para ejecutar: MySQL, Redis, Sidekiq y (por supuesto) el servidor d...

4  ¿Hay una manera 'mejor' de encontrar archivos de una lista en un árbol de directorios  ( Is there a better way to find files from a list in a directory tree ) 
He creado una lista de archivos usando find , 9988776655544336 . El comando Buscar es simplemente find . -type f -name "<search_pattern>" > foundlist.lst...

5  Lista de uso del disco del directorio  ( Directorys disk usage list ) 
Para mis clases, tuve que terminar esta tarea: Lista de uso del disco de directorio para el directorio indicado Imprime una lista de archivos y subdire...

2  Fusionar directorios y mantener archivos que tengan más líneas  ( Merging directories and keep files that have more lines ) 
gol Mi objetivo es combinar directorios. Cada vez que un archivo tiene el mismo nombre en dos o más directorios, solo se debe mantener el número más alto ...

3  Algoritmo de copia de seguridad incremental rápido usando RSYNC  ( Fast incremental backup algorithm using rsync ) 
Estoy ejecutando un script de bash simple que usa rsync para realizar una copia de seguridad incremental de mi servidor web cada hora. Lo que estoy buscando...

6  SymLinking Bibliotecas compartidas de un directorio específico de la plataforma  ( Symlinking shared libraries from a platform specific directory ) 
Tenemos un script de configuración para configurar un entorno de desarrollo, y configurar las bibliotecas de imágenes se está metiendo un poco: IndexOf7 ...

2  Comparando si es, y condición de caso  ( Comparing if else and case condition ) 
Tengo un script utilizado para instalar pocos rpms dependiendo del tipo de sistema operativo. Aquí, en este ejemplo, el script instalará paquetes en dos "Cent...

2  Resolver las dependencias de la producción del sistema de cola PBS  ( Resolve dependencies from output of pbs queueing system ) 
He escrito un script, que reformate la salida de qstat -f1 de la pbs sistema de cola . Desafortunadamente, este proyecto es demasiado grande para publicar ...

6  Cómo mejorar mi script de Python de reconexión automática  ( How to improve my auto reconnection python script ) 
Necesito algunos comentarios aquí. Debido a un problema extraño en mi sistema de servidores, la tarjeta de red se desconecta de la red regularmente. Lo arregl...




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