[poxitron]

Hace tiempo que llevo dándoles vueltas a esto y no sé muy bien cómo poner este código de PatchMe. Lo que hace es comprobar si está marcada la casilla de "Usar nombre para .xdelta" y si la etiqueta "origen/destino" es visible para decidir qué nombre poner a los archivos generados por xdelta.

El caso es que se me ocurrió dos formas de ponerlo:
1. Colocar las dos comprobaciones de forma que se realicen cuando se vaya a procesar cada uno de los archivos, escribiendo así menos código.

spoiler:

Mostrar

Una pequeña duda sobre programación 561579a75c

2. Colocar una de las comprobaciones fuera para que no se realice por cada archivo a procesar, lo que hace que haya más código, casi todo un copia y pega.

spoiler:

Mostrar

Una pequeña duda sobre programación 06138cbb56

¿Qué opináis? ¿Es mejor la segunda opción, sacar una de las comprobaciones y escribir más código, o creéis lo que la cpu tarda en hacer las comprobaciones es tan insignificante que es mejor la primera opción, la de escribir menos código?

¿O se os ocurre alguna otra forma de ponerlo que quede mejor?

[Kumicho]

Solo es más código porque haces todo inline en vez de llamar una función. Dicho esto, el tiempo que se gasta en hacer la comprobación es tan poco que no tiene importancia.
Si te preocupes por la velocidad deberías quitar el count del loop - no recuerdo bien Delphi, pero si es una función -como count() en C#- y no un variable o property (¿característica?) -como length en C#- invocar la misma función un montón de veces sí cuesta un montón de ciclos, aunque aún así, teniendo en cuenta el número de archivos que el usuario va a procesar, no tiene importancia.

No he visto tu aplicación, pero a no ser que cuando el usuario cambia el valor del combobox verifiques que sea valido, el buffer puede ser 0.

[poxitron]

El Items.Count es una property con la que se recorren todos los items de un listbox, no la puedo quitar. Y en cuanto a lo del buffer, si te refieres a lo de "buffer := '-B' + IntToStr(b);" es simplemente un parámetro seleccionable por el usuario (que debo convertir a integer para pasarlo a bytes y luego convertirlo a una string) que le paso a xdelta.

Pero bueno, si tú también crees que el tiempo que tarda en hacer la comprobación es poquísimo, lo haré como en la primer opción y me ahorro código.

Gracias, Kumicho.

[Kumicho]

(17 Jul 2015)poxitron escribió:  El Items.Count es una property con la que se recorren todos los items de un listbox, no la puedo quitar.

Claro que puedes. Puedes almacenar su valor en una variable y usar esa variable en la iteración. El beneficio depende de como está programado el "get" del property - o sea de si tiene que leer una variable privada o si tiene que contar, pero en ambos casos ganas.

(17 Jul 2015)poxitron escribió:  "buffer := '-B' + IntToStr(b);" es simplemente un parámetro seleccionable por el usuario (que debo convertir a integer para pasarlo a bytes y luego convertirlo a una string) que le paso a xdelta.

b := StrToInt(Form1.Buffer_ComboBox.Text) * 1048576
por lo tanto si Buffer_ComboBox.Text = 0, b = 0; no tengo ni idea de qué hace xdelta cuando especifiques un buffer de 0, pero no me parece correcto.

[poxitron]

(17 Jul 2015)Kumicho escribió:  

(17 Jul 2015)poxitron escribió:  El Items.Count es una property con la que se recorren todos los items de un listbox, no la puedo quitar.

Claro que puedes. Puedes almacenar su valor en una variable y usar esa variable en la iteración. El beneficio depende de como está programado el "get" del property - o sea de si tiene que leer una variable privada o si tiene que contar, pero en ambos casos ganas.

Pues es la única forma que he visto por internet para hacer un loop en un listbox y luego hacer algo con cada uno de los items.
Esto es lo que dice la documentación:

spoiler:

Mostrar

Una pequeña duda sobre programación 3025b82a3f

Si conoces una forma mejor de hacer, soy todo oídos. Ten en cuenta que yo no sé casi nada de programación.

(17 Jul 2015)Kumicho escribió:  

(17 Jul 2015)poxitron escribió:  "buffer := '-B' + IntToStr(b);" es simplemente un parámetro seleccionable por el usuario (que debo convertir a integer para pasarlo a bytes y luego convertirlo a una string) que le paso a xdelta.

b := StrToInt(Form1.Buffer_ComboBox.Text) * 1048576
por lo tanto si Buffer_ComboBox.Text = 0, b = 0; no tengo ni idea de qué hace xdelta cuando especifiques un buffer de 0, pero no me parece correcto.

El Buffer_ComboBox ya lleva unos cuantos valores que son los más usados, si el usuario es tan listo como para borrarlo y poner un 0, allá él. Además, en los requisitos mínimos ya especifico que hace falta un cerebro xD

[Kumicho]

var count;
count := Form1.Origen_ListBox.item.count - 1;
for i := 0 to count do

Aunque lo cierto es que con el tiempo que tarda el proceso xdelta este tipo de optimizaciones son innecesarios; y más teniendo en cuenta que seguramente el listbos es virtual.
Hay más optimizaciones que puedes hacer, pero son todos de este tipo, o sea que si no vas a hacer profiling no notarás ninguna diferencia.

[7l6j1A]

Parce que le tienes miedo a los "if" anidados... y ni como culparte, en Delphi son especialmente confusos y hay que tener cuidado con los "end", solo existe un "end;".
Se pueden optimizar muchas cosas, como la condición del "for", eliminar o reciclar variables, etc.
Mientras más simple sea el código, mejor.

Ejem:

Mostrar

Código:

procedure Ejecutarxdelta;
var
b, i, n: integer;
buffer, parametros, ejecutable: string;
begin
  n := 0;
  b := StrToInt(Form1.Buffer_ComboBox.Text) * 1048576;
  buffer := '-B' + IntToStr(b);
  for i := 0 to Form1.Origen_ListBox.Items.Count - 1 do
    begin
      ArchivoOrigen := Form1.Origen_ListBox.Items.Strings[i];
      ArchivoDestino := Form1.Destino_ListBox.Items.Strings[i];
      if Form1.xdeltaFileNames_CecckBox.Checked = True then
        if Form1.xdeltaFileNamesDestino_Label.Visible = True then
          Archivoxdelta := RutaParche + ExtractFileNameOnly(ArchivoDestino) + '.xdelta'
        else
          Archivoxdelta := RutaParche + ExtractFileNameOnly(ArchivoOrigen) + '.xdelta';
      else
        begin
          inc(n);
          Archivoxdelta := RutaParche + 'file' + AddLeadingZeroes(n, 2) + '.xdelta';
        end;
      end;
      parametros := buffer + ' -f -e -s "' + ArchivoOrigen + '" "' + ArchivoDestino + '" "' + Archivoxdelta + '"';
      ExecNewProcess(RutaEjecutable + '\xdelta.exe ' + parametros, SW_HIDE, True);
    end;
end;

Nunca use ListBox, pero si tiene la propiedad First, Eof, etc. podrías desechar la variable i y cambiar el "for" por un "while" o un "repeat".

[Kumicho]

Reciclar variables no suele mejorar mucho la eficiencia, porque el compilador ya se encarga de eso.

En vez de "concatar" tantos strings es mucho más eficiente usar Format. Cada + crea una nueva copia de todo el string en la memoria. (el compilador podría hacerlo, pero por lo menos antes no lo hacía)

Como Form1.xdeltaFileNames_CecckBox.Checked no cambiará durante el loop es mejor guardar el resultado en una variable local.
Lo mismo para Form1.xdeltaFileNamesDestino_Label.Visible

Iba a recomendar otra cosa, pero veo que Delphi sigue sin tener un ternary operator (a = x ? y : z) Ya recuerdo por qué dejé de usar Delphi (hace falta mucho más código para hacer lo mismo que en C#)

[poxitron]

(21 Jul 2015)7l6j1A escribió:  Parce que le tienes miedo a los "if" anidados... y ni como culparte, en Delphi son especialmente confusos y hay que tener cuidado con los "end", solo existe un "end;".

No les tengo ningún miedo a los "if then", lo que me preocupaba era lo poco eficiente que podría ser meter las dos comprobaciones en el loop.

(21 Jul 2015)Kumicho escribió:  En vez de "concatar" tantos strings es mucho más eficiente usar Format. Cada + crea una nueva copia de todo el string en la memoria. (el compilador podría hacerlo, pero por lo menos antes no lo hacía)

Como Form1.xdeltaFileNames_CecckBox.Checked no cambiará durante el loop es mejor guardar el resultado en una variable local.
Lo mismo para Form1.xdeltaFileNamesDestino_Label.Visible

Te he haré caso con el Format, pero ¿cómo sería lo de guardar el resultado del checkbox y del label en una variable? ¿No tendría que hacer igualmente la comprobación de su contenido igual que hago ahora para saber qué guardo en la variable Archivoxdelta?

[Kumicho]

Copio el código de 7l6j1A, supongo que es igual al tuyo:

spoiler:

Mostrar

procedure Ejecutarxdelta;
var
b, i, n: integer;
LabelVisible, CheckBoxChecked: boolean;
buffer, parametros, ejecutable: string;
begin
n := 0;
b := StrToInt(Form1.Buffer_ComboBox.Text) * 1048576;
buffer := '-B' + IntToStr(b);
LabelVisible := Form1.xdeltaFileNamesDestino_Label.Visible;
CheckBoxChecked := Form1.xdeltaFileNames_CecckBox.Checked
for i := 0 to Form1.Origen_ListBox.Items.Count - 1 do
begin
ArchivoOrigen := Form1.Origen_ListBox.Items.Strings[i];
ArchivoDestino := Form1.Destino_ListBox.Items.Strings[i];
if CheckBoxChecked = True then
if LabelVisible = True then
Archivoxdelta := RutaParche + ExtractFileNameOnly(ArchivoDestino) + '.xdelta'
else
Archivoxdelta := RutaParche + ExtractFileNameOnly(ArchivoOrigen) + '.xdelta';
else
begin
inc(n);
Archivoxdelta := RutaParche + 'file' + AddLeadingZeroes(n, 2) + '.xdelta';
end;
end;
parametros := buffer + ' -f -e -s "' + ArchivoOrigen + '" "' + ArchivoDestino + '" "' + Archivoxdelta + '"';
ExecNewProcess(RutaEjecutable + '\xdelta.exe ' + parametros, SW_HIDE, True);
end;
end;