8
respuestas

[Sugerencia] Mejora Al código

Les comparto el código con un array para que el color del circulo sea random.

Ingrese aquí la descripción de esta imagen para ayudar con la accesibilidadeste seria el resultado Ingrese aquí la descripción de esta imagen para ayudar con la accesibilidad

8 respuestas

Hola muy buena sugerencia, gracias.

Creo que no hace falta poner el Meth.round, ya que no hay ningún número que redondear. Saludos

Excelente Mario!

Es correcto el uso de Math.round para hacer solo valores enteros dado que math.random regresa valores punto flotante entre 0 y 1, muy buen manejo de JavaScript! se nota que tu si sabes mas que los de platzi

en lo personal le hubiera pasado colores Neon en formato hexadecimal para que se vea mas fancy, jeje

Todo desatinado el comentario de Platzi del compañero José Domingo Pillado Chavarría pero bueno. Acá te dejo una versión sin el Math.round:

  let color = ["blue", "green", "gold", "red"];



    function dibujarCirculo(evento){
        let x = evento.pageX - pantalla.offsetLeft;
        let y = evento.pageY - pantalla.offsetTop;
        pincel.fillStyle = color[(Math.floor(Math.random() * (color.length)))];
        pincel.beginPath();
        pincel.arc(x, y, 10, 0, 2 * 3.14);
        pincel.fill();
        console.log(evento);



    }

En lugar de multiplicar por cuatro, multiplicas por la totalidad del array, es decir, .lenght así puedes agregar los colores que quieras y no hace falta modificar el Math de nuevo.

Sugerencias en todas las respuestas en este comentario. Sugerencia 1: Mario García: La funcion mostrarMensaje no es necesario en el codigo, redunda codigo. Lo otro es que para poner codigo de manera de respuesta en una de las opciones hay un icono como este "</>" le haces clic y dentro pones tu codigo. o dentro de estas comillas. Sugerencia 2: Mario García, Ángela Sofía: les recomiendo este codigo que no usa array, pero si usa una funcion que crea rgb por color de modo aleatorio, es decir, crea puntos de cualquier color aleatoriamente, les dejo el codigo:

<canvas width="600" height="400"></canvas>

<script>

    var pantalla = document.querySelector("canvas");
    var pincel = pantalla.getContext("2d");

    pincel.fillStyle = "grey";
    pincel.fillRect(0,0,600,400);  

    function colorAleatorio() {
        var red = Math.floor(Math.random() * 256);
        var green = Math.floor(Math.random() * 256);
        var blue = Math.floor(Math.random() * 256);
        var color = "rgb(" + red + ", " + green + ", " + blue + ")";
        return color;         
    }

    function dibujarCirculo(evento) {
        var x = evento.pageX - pantalla.offsetLeft;
        var y = evento.pageY - pantalla.offsetTop;
        pincel.fillStyle=colorAleatorio();
        pincel.beginPath();
        pincel.arc(x,y,10,0,2*3.14);
        pincel.fill();
        console.log(x + ","+y);
    }

    pantalla.onclick=dibujarCirculo;
</script>

Ingrese aquí la descripción de esta imagen para ayudar con la accesibilidad

Aunque me gusta mas la propuesta de George:

  1. Si vas a declarar variables de mismo valor es mejor la sintaxis var[] = [];.
  2. Puedes declarar una const global para el parámetro Math.floor... y pasarselo al momento de declara var: const ev = Math.floor(Math.random() * 256); var[red, green] = [ev,ev];.
  3. Puedes usar return y cadenas de texto para el formato rgb: return `rgb(${red},${green},${blue}):
  4. puedes refactorizar pantalla y pincel así: const [pantalla, pincel] = document.querySelector("canvas").getContext("2d");.
  5. Es cierto la redundancia de mostrarMensaje, sin embargo aunque imprimir mensajes es bueno en la etapa de desarrollo para hacer debug, no se deben incluir en la versión final.

Muy buen aporte Jóse, Saludos

al contrario George, cuando puedes refactorizar tal cual tu lo hiciste significa que ya vas dominando la programación!