gabriel9216 / skrypt_temperatury

0 stars 0 forks source link

Code review #3

Open niedzielnyaniol opened 1 year ago

niedzielnyaniol commented 1 year ago

Convention:

<!DOCTYPE html>
<html lang="en">
// You should indent your code - head tag is inside html tag so it should be moved by one tab.
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Przelicznik temperatur</title>
</head>
// code indentation
<body>
    <h1>strona przeliczająca temperatury</h1>
    <hr>
    <input type="number" id="number" placeholder="podaj temperature">
    <h2>wybierz na jaka jednostke chcesz ja przeliczyc</h2>
    <button onclick="celc()">przelicz na celcjusze</button>
    <button onclick="fahr()">przelicz na fahrenheity</button>
        // unnecessary space
    <div id="temp" ></div>

    <script>
                // `celc` what? Use noun to name function. Also name should be more descriptive
        function celc()
        {
                // https://medium.com/javascript-scene/javascript-es6-var-let-or-const-ba58b8dcde75#:~:text=%60const%60%20is%20a%20signal%20that,always%20the%20entire%20containing%20function.
            let number=document.getElementById('number').value;
            // `1.8` and `32` are "magic numbers"
            let cel=number*1.8+32;
            let wynik=document.getElementById('temp');
            // never use ==. === is faster and better. https://www.c-sharpcorner.com/article/avascript-type-coercion-explained-how-to-avoid-common-pitfalls/#:~:text=In%20JavaScript%2C%20type%20coercion%20happens,and%20concatenate%20the%20two%20values.
            if(number==1){
                    // use template literals to combine string. Why you use `round`?
                wynik.innerHTML=number+" stopień celcjusza to "+Math.round(cel)+" fehrenheitów";
            } else
                                 // DRY - this line is almost the same like in "if"
                wynik.innerHTML=number+" stopnie celcjusza to "+Math.round(cel)+" fehrenheitów";
        }
        function fahr()
        {
                        // I saw that line before. Move it to function. Eg `getNumberInputValue`
            let number=document.getElementById('number').value;
                        // fah what? Magic numbers
            let fah=(number-32)/1.8;
            // i saw this line before
            let wynik=document.getElementById('temp');
            // Magic numbers. == instead of ===
            if(number==34 || number==33){
                wynik.innerHTML=number+" fehrenheity to "+Math.round(fah)+" stopień celcjusza";
            } else
                wynik.innerHTML=number+" fehrenheitów to "+Math.round(fah)+" stopni celjusza";
// wrong indent
        }
    </script>
</body>
</html>

Also:

gabriel9216 commented 11 months ago

done