Que opinan de mi codigo y como mejorarlo

[th3r0rn]

Hola, m gustaria le hecharan un vistaso a mi codigo y me dijeran que puedo mejorar de el ya que yo lo siento muy "sucio".
Explico:
Este codigo lo que haxce es ejecutar un procedure que busca datos por tiempo real, es decir la hora actual, pero si no se encuentran datos en esa hora lo que hace es cerrar la conexion con la DB y abrir otra para consultar datos por la hora anterior actual, y ara esto siempre y cuando no encuentre datos en la hora actual:

Código ASP:

'########################################## L15 SUR #######################################################################################
    linea15s="L15s"
        SET conexion15s=Server.CreateObject("ADODB.Connection")
    conexion15s.Open "PROVIDER=SQLOLEDB; DATA SOURCE=VALP-MESSRV02;UID=sa;PWD=messrv;DATABASE=wasp "
            SET registros15s=Server.CreateObject("ADODB.RecordSet")
         registros15s.open"EXEC monitor '"&fecha&"','"&linea15s&"','"&horamia&"'",conexion15s
        
         SET conexionAVG15s=Server.CreateObject("ADODB.Connection")
    conexionAVG15s.Open "PROVIDER=SQLOLEDB; DATA SOURCE=VALP-MESSRV02;UID=sa;PWD=messrv;DATABASE=wasp "
            SET registrosAVG15s= Server.CreateObject("ADODB.RecordSet")
        
        registrosAVG15s.open"SELECT AVG(PESO)as promedio,o.linea from Optimas as o INNER JOIN pesos as p on o.codigo =p.codoptima WHERE fecha >='"&fecha&"' AND hora='"&horamia&"' AND o.linea='L15S' GROUP BY LINEA",conexionAVG15s
    
        do while not registrosAVG15s.eof
        promedio=cint(registrosAVG15s.fields("promedio"))
                
    if registros15s.eof then
    conexion15s.close
    SET conexion15s=Server.CreateObject("ADODB.Connection")
    conexion15s.Open "PROVIDER=SQLOLEDB; DATA SOURCE=VALP-MESSRV02;UID=sa;PWD=messrv;DATABASE=wasp "
            SET registros15s=Server.CreateObject("ADODB.RecordSet")
    registros15s.open"EXEC monitor '"&fecha&"','"&linea15s&"','"&hora2&"'",conexion15s
        response.write("EXEC monitor '"&fecha&"','"&linea15s&"','"&hora2&"'")
        do while not registros15s.eof
        
                    target=cint(registros15s.fields("target"))'## Definicion de variable Target
                    OP=(promedio-target)/target  '### Definicion de operacion de la variable OP para ser mostrada en la tabla
                    saving=(PROMEDIO-target)*NumBolsas '### Definicion y operacion de la variable SAVING para mostrar en la tabla
        response.write("<tr>")
        response.write("<td>"&registros15s.fields("linea")&"</td>")
        response.write("<td>"&registros15s.fields("peso")&"</td>")
        response.write("<td>"&registros15s.fields("target")&"</td>")
        response.write("<td>"&registros15s.fields("marca")&"</td>")
        response.write("<td>"&registros15s.fields("vaso")&"</td>")
        'response.write("<td>"&registros6n.fields("fecha")&"</td>")
        response.write("<td>"&fechahoy&" "&hora&":"&minuto&"</td>") 
        response.write("<td><h3>"&registros15s.fields("totality")&"</h3></td>")
        response.write("<td>"&registrosAVG15s.fields("promedio")&"</td>")
        if OP<=0 then response.write("<td>"&op&"<img src='http://www.pact-oil.co.uk/images/error.gif'</a></td>") else response.write("<td>"&op&"<img src='http://www.hospitalsierrallana.com/imagenes/gif_ok.gif'</a></td>") end if
            if saving <0 then
        response.write("<td bgcolor= '#FF0000'>  "&saving&" KG</td>")   
        elseif saving >0 then   
        response.write("<td bgcolor='#00FF00'>"&saving&" KG</td>") 
        elseif saving=0 then
        response.write("<td>"&saving&" KG</td>") 
        end if
        response.write("</tr>")
registros15s.movenext
    loop
        
        conexion15s.close
    else
    
        do while not registros15s.eof
        
                    target=cint(registros15s.fields("target"))'## Definicion de variable Target
                    OP=(promedio-target)/target  '### Definicion de operacion de la variable OP para ser mostrada en la tabla
                    saving=(PROMEDIO-target)*NumBolsas '### Definicion y operacion de la variable SAVING para mostrar en la tabla
        response.write("<tr>")
        response.write("<td>"&registros15s.fields("linea")&"</td>")
        response.write("<td>"&registros15s.fields("peso")&"</td>")
        response.write("<td>"&registros15s.fields("target")&"</td>")
        response.write("<td>"&registros15s.fields("marca")&"</td>")
        response.write("<td>"&registros15s.fields("vaso")&"</td>")
        'response.write("<td>"&registros6n.fields("fecha")&"</td>")
        response.write("<td>"&fechahoy&" "&hora&":"&minuto&"</td>") 
        response.write("<td><h3>"&registros15s.fields("totality")&"</h3></td>")
        response.write("<td>"&registrosAVG15s.fields("promedio")&"</td>")
        if OP<=0 then response.write("<td>"&op&"<img src='http://www.pact-oil.co.uk/images/error.gif'</a></td>") else response.write("<td>"&op&"<img src='http://www.hospitalsierrallana.com/imagenes/gif_ok.gif'</a></td>") end if
            if saving <0 then
        response.write("<td bgcolor= '#FF0000'>  "&saving&" KG</td>")   
        elseif saving >0 then   
        response.write("<td bgcolor='#00FF00'>"&saving&" KG</td>") 
        elseif saving=0 then
        response.write("<td>"&saving&" KG</td>") 
        end if
        response.write("</tr>")
registros15s.movenext
    loop
        
        conexion15s.close
        
end if
registrosAVG15s.movenext
    loop
        
        conexionAVG15s.close
 

Saludos

[Myakire]

A ojo de buen cubero:

1) No usas ningún estilo de notación
2) No intercalas la parte de ASP con el HTML, eso es menos lento que solo llamar el método .write de response.
3) Creas objetos de forma innecesaria
4) No reutilizas código, lo copias tanto por el IF como por el ELSE

[th3r0rn]

Hola Myakire Muchas gracias por responder y darme tus consejos ahora bien tengo un par de cuestiones respecto a lo que dices:

1) No usas ningún estilo de notación
A que tipo de notacion te refieres? como para que o q?

2) No intercalas la parte de ASP con el HTML, eso es menos lento que solo llamar el método .write de response.

De esto no entiendo, como intercalarlo con HTML?

3) Creas objetos de forma innecesaria
Aqui me comentas sobre objetos que creo de forma innecesaria, cuales? los objetos que creo solamente son los de la hora.

4) No reutilizas código, lo copias tanto por el IF como por el ELSE
En esto estoy muy deacuerdo, de hecho es lo que mas he notado, que me recomiendas, usar metodos o funciones? para solo mandar a llamar la parte del codigo donde imprimo los resultados?
Saludis

[Myakire]

Ok, te paso a explicar de forma breve cada punto.

1) La notación es un estilo general para nombrar a tus variables, existen varios estilos de notación y ayudan para diferentes cosas, desde mejorar la legibilidad, hasta saber el tipo o contenido de una variable. Algunos lenguajes incluso tienen definidas convenciones de notación mnemónica que bien podrías aplicar en tus códigos como guía.

2) El intercalar ASP y HTML se refiere básicamente en abrir y cerrar las etiquetas <% y %> para el código del servidor y separar lo del cliente, es un estilo viejo de script donde en un mismo archivo mezclas ambas programaciones pero en ASP 3.0 es lo que se estila, puedes separar más la capa de negocios (entiéndase las reglas que el programa sigue) de la de presentación (sería el HTML en este caso) si usas DLL's, pero en tu caso no creo sea necesario. El generar la salida HTML con response.write solo hace más lenta la generación del HTML resultante y por ende tu página se ve más tarde en el browser.

3) Los objetos conexion15s y registros15s los creas dos veces y peor lo haces dentro de un ciclo, esta destrucción implícita de objetos consume más tiempo que si solo cierras el recordSet y lo abres con la nueva consulta (no he visto si es realmente necesario hacer eso, no he visto más que por encima tu código).

4) Un método y una función básicamente es lo mismo, quizá te refieres a una subrutina (o procedimiento) y a una función.
Las funciones regresan un valor y las subrutinas no, por lo poco que veo considero que una subrutina sería suficiente.

Saludos

[th3r0rn]

Hola, muchas gracias por explicarme, sobre la subrutina creo que seria mejor una funcion porque tengio pensado hacer los mismos para todos los casos de la aplicacion aqui todo su code: http://pastebin.com/42ZqwWc6
Saludos

[Adler]

Hola

Al leer la explicación que das de lo que hace el código me llamó la atención. Ojeando el código no veo como logras hacer

Este codigo lo que haxce es ejecutar un procedure que busca datos por tiempo real, es decir la hora actual, pero si no se encuentran datos en esa hora lo que hace es cerrar la conexion con la DB y abrir otra para consultar datos por la hora anterior actual, y ara esto siempre y cuando no encuentre datos en la hora actual

¿Estás usando Ajax con un temporizador, COMET? ¿o son simple consultas que se ejecuta cada vez que el usuario entra en tu web?

[th3r0rn]

Hola, nop no uso ajax solo refresco la pagina cada segundo, y solo cambian los valores que hay, es decir si hay 60 hace nuevamente lña consuylta y se suman los valores agregados y cambioa a 80 y asi durante todo el dia ya que a la base de datos se estan insertando alrededor de 500 valores por segundo mas o menos

[Adler]

Hola

Pues eso ha de consumir un montón de recursos por no decir lo molesto que ha de ser que cada segundo ses esté refrescando la página. No puedo hablarte casi que nada de COMET, asignatura pendiente, pero es justamente lo que necesitas. Abre una conexión en segundo plano y se queda abierta a la espera de que haya algún cambio. Es como lo que te comentaba de AJAX pero el polling es menor. Una buena esplicación la da @Panino en la faq de AJAX con un mini chat

Suerte