1
\$\begingroup\$

This is a work in progress Rubik's Cube scramble generator with timer and list of solves. The list of solves is stored using localstorage so that it stays on page reload. A solve can also be deleted from the list. I used some basic boostrap for alignment and buttons. I also used jQuery to make the list of solves. I am planning to add customization and a "settings" panel. Any feedback/suggestions for improvement appreciated!

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Speedcubing Timer</title>
    <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/[email protected]/dist/css/bootstrap.min.css" integrity="sha384-ggOyR0iXCbMQv3Xipma34MD+dH/1fQ784/j6cY/iJTQUOhcWr7x9JvoRxT2MZw1T" crossorigin="anonymous">
    <style>
        * {
            padding:auto;
        }
        body {
            padding:20px;
        }
        #times-list-template {
            height:20vh;
            max-height: 20vh;
            overflow-y: auto;
        }
        .hidden {
            display:none;
        }
        #timer-info-box {
            position:fixed;
            bottom:90vh;
            height:10vh;
            background-color:gray;
            padding:5px;
        }
        #minutes, #seconds, #seconds-placeholder {
            margin:none;
        }
        .col-xs-5 {
            padding: 20px;
        }
        
    </style>
</head>
<body>
<h1>CubeTimer v1.0</h1>
    <div class="row">
        <div class="col-xs-5">
            <h1>Scramble</h1>
            <h3 id="scramble"></h1>
            <button onclick="rescramble()" class="btn btn-primary">Rescramble</button> 
        </div>
        <div class="col-xs-5">
            <h1>Timer<i class="fa-solid fa-circle-info" id="timer-info"></i></h1>
            <h3 id="timer"><span id="minutes"></span>:<span id="seconds-placeholder">0</span><span id="seconds"></span>.<span id="tenths"></span></h3>
            <button id="start-timer" class="btn btn-primary">Start Timer</button>
            <button onclick="list_time()" class="btn btn-secondary">Save to list</button>
        </div>
        <div class="col-xs-5">
            <div id="timer-info-box" class="hidden"><p>Press the spacebar to start the timer!</p></div>
            <h1>Times</h1>
            <div id="times-list"><ol id="times-list-template"></ol></div>
        </div>
    </div>
<script src="https://code.jquery.com/jquery-3.3.1.slim.min.js" integrity="sha384-q8i/X+965DzO0rT7abK41JStQIAqVgRVzpbzo5smXKp4YfRvH+8abtTE1Pi6jizo" crossorigin="anonymous"></script>
<script src="https://kit.fontawesome.com/c63b6d877c.js" crossorigin="anonymous"></script>
<script>
let scramble = Math.floor(Math.random() * 12);
function scramble_translator() {
    let scramble = Math.floor(Math.random() * 12);
    if (scramble <= 1) {
        scramble = 'R '
    } else if (scramble > 1 && scramble <= 2) {
        scramble = 'R\' '
    } else if (scramble > 2 && scramble <= 3) {
        scramble = 'U '
    } else if (scramble > 3 && scramble <= 4) {
        scramble = 'U\' '
    } else if (scramble > 4 && scramble <= 5) {
        scramble = 'L '
    } else if (scramble > 5 && scramble <= 6) {
        scramble = 'L\' '
    } else if (scramble > 6 && scramble <= 7) {
        scramble = 'F '
    } else if (scramble > 7 && scramble <= 8) {
        scramble = 'F\' '
    } else if (scramble > 8 && scramble <= 9) {
        scramble = 'D '
    } else if (scramble > 9 && scramble <= 10) {
        scramble = 'D\' '
    } else if (scramble > 10 && scramble <= 11) {
        scramble = 'B '
    } else if (scramble > 11 && scramble <= 12) {
        scramble = 'B\' '
    }

    document.getElementById('scramble').innerHTML += scramble;
}
for (let i = 0; i < 20; i++) {
    scramble_translator();
}
document.getElementById('seconds').innerHTML = 0;
document.getElementById('minutes').innerHTML = 0;
document.getElementById('tenths').innerHTML = 0;
let intervalId;
let tenthsInterval;
let minutes = 0;
let seconds = 0;
let tenths = 0;
let time_lister = 1; // Move the variable declaration outside the function

function timer() {
    document.addEventListener('keydown', function(event) {
        if (event.key === ' ') {
            if (tenths === 0 && seconds === 0 && minutes == 0) {
                intervalId = setInterval(function() {
                    seconds++;
                    document.getElementById('seconds').innerHTML = seconds;
                
                    if (seconds > 59) {
                        minutes++;
                        document.getElementById('minutes').innerHTML = minutes;
                        seconds = 0;
                        document.getElementById('seconds').innerHTML = seconds;
                    }
                    if (seconds >= 10) {
                        document.getElementById('seconds-placeholder').classList.add('hidden');
                    }
                    if (seconds < 10) {
                        document.getElementById('seconds-placeholder').classList.remove('hidden');
                    }
                }, 1000);
                tenthsInterval = setInterval(function() {
                    tenths++;
                    document.getElementById('tenths').innerHTML = tenths;

                    if (tenths > 9) {
                        tenths = 0;
                        document.getElementById('tenths').innerHTML = tenths;
                    }
                }, 100);
            } else {
                clearInterval(intervalId);
                clearInterval(tenthsInterval);
            }
        }
    });
    document.getElementById('start-timer').addEventListener('click', function(event) {
            document.getElementById('start-timer').innerHTML = 'Stop timer';
            if (tenths === 0 && seconds === 0 && minutes == 0) {
                intervalId = setInterval(function() {
                    seconds++;
                    document.getElementById('seconds').innerHTML = seconds;
                
                    if (seconds > 59) {
                        minutes++;
                        document.getElementById('minutes').innerHTML = minutes;
                        seconds = 0;
                        document.getElementById('seconds').innerHTML = seconds;
                    }
                    if (seconds >= 10) {
                        document.getElementById('seconds-placeholder').classList.add('hidden');
                    }
                    if (seconds < 10) {
                        document.getElementById('seconds-placeholder').classList.remove('hidden');
                    }
                }, 1000);
                tenthsInterval = setInterval(function() {
                    tenths++;
                    document.getElementById('tenths').innerHTML = tenths;

                    if (tenths > 9) {
                        tenths = 0;
                        document.getElementById('tenths').innerHTML = tenths;
                    }
                }, 100);
            } else {
                clearInterval(intervalId);
                clearInterval(tenthsInterval);
            }
        
    });
}
timer();
let timesList = '';

function list_time() {
    document.getElementById('start-timer').innerHTML = 'Start timer';
    let new_time = $('#timer').html();
    let elementId = 'time' + time_lister;
    $('<div id="' + elementId + '">').html(new_time).appendTo('#times-list-template');
    $('<button style="margin-bottom:10px" class="btn btn-danger" onclick="delete_(\'' + elementId + '\')">').html('Delete').appendTo('#' + elementId);
    time_lister++;
    document.getElementById('scramble').innerHTML = '';
    for (let i = 0; i < 20; i++) {
        scramble_translator();
    };
    seconds = 0;
    minutes = 0;
    tenths = 0;
    document.getElementById('minutes').innerHTML = minutes;
    document.getElementById('seconds').innerHTML = seconds;
    document.getElementById('tenths').innerHTML = tenths;

    // Update the timesList variable with the updated times
    timesList = document.getElementById('times-list-template').innerHTML;

    // Store the list of times in local storage
    localStorage.setItem('timesList', timesList);
}

function delete_(elementId) {
    $('#' + elementId).remove();

    // Update the timesList variable with the updated times
    timesList = document.getElementById('times-list-template').innerHTML;

    // Store the list of times in local storage
    localStorage.setItem('timesList', timesList);
}

document.getElementById('timer-info').addEventListener('mouseover', (event) => {
    document.getElementById('timer-info-box').classList.remove('hidden');
});
document.getElementById('timer-info').addEventListener('mouseout', (event) => {
    document.getElementById('timer-info-box').classList.add('hidden');
});

document.addEventListener('keydown', (event) => {
    if (event.key === 'Enter') {
        list_time();
    }
});
function rescramble() {
    document.getElementById('scramble').innerHTML = '';
    for (let i = 0; i < 20; i++) {
        scramble_translator();
    };
}

// Retrieve the list of times from local storage on page load
document.addEventListener('DOMContentLoaded', function() {
    timesList = localStorage.getItem('timesList');
    if (timesList) {
        document.getElementById('times-list-template').innerHTML = timesList;
    }
});


</script>
</body>
</html>
\$\endgroup\$
5
  • \$\begingroup\$ Is there any specific reason you used outdated Bootstrap 4.3 (not even 4.6) instead of Bootstrap 5.3? Also any specific reasons to mix JavaScript and jQuery? \$\endgroup\$
    – tacoshy
    Commented Nov 25, 2023 at 21:51
  • \$\begingroup\$ For starters, you can get rid of your low-performing if/else statement meanance and randomize from an array: codepen.io/tacoshy/pen/ExrRdVx?editors=1111 \$\endgroup\$
    – tacoshy
    Commented Nov 25, 2023 at 22:04
  • \$\begingroup\$ codepen.io/tacoshy/pen/xxMzoWY - I just rewrote your scramble logic which takes about 1/500th of the time to execute than your logic. \$\endgroup\$
    – tacoshy
    Commented Nov 26, 2023 at 16:09
  • \$\begingroup\$ @tacoshy Consider posting an answer with the code included as a runnable snippet, rather than codepen links that can rot over time. \$\endgroup\$
    – ggorlen
    Commented Nov 26, 2023 at 20:16
  • \$\begingroup\$ @ggorlen unfortunatly I do not have the time to do a review to my quality standards, that's why I rather post single small parts as a codepen. Maybe some other reviewer fin it useful enough to use it for his review. \$\endgroup\$
    – tacoshy
    Commented Nov 26, 2023 at 21:31

2 Answers 2

2
\$\begingroup\$

I do not have the time to do a review to my quality standards

I do not presume to eclipse your star-shine quality standards, rather, in the spirit of of teamwork I offer my modest input based on @tacoshy's guttentight coding.

Look for Structures

... such as Array, Map, Set, Object, etc. (or make your own). This is an active, deliberate, imaginative thought process. This linear numeric sequence is like Array indexes.

if (scramble <= 1) {
    scramble = 'R '
} else if (scramble > 1 && scramble <= 2) {
    scramble = 'R\' '
} else if (scramble > 2 && scramble <= 3) {
    scramble = 'U '
} else if (scramble > 3 && scramble <= 4) {

Then we take advantage of Array's functionality. A coding insight in this case is execution/operator precedence - an expression in [ ] will be evaluated before it is used as the index. No intermediate variables needed.

const SCRAMBLES = ['F', 'B', 'L', 'R', 'T', 'U']
 return SCRAMBLES[Math.floor(Math.random() * SCRAMBLES.length)];

The above also takes advantage of the Math object. How did he know to do that?

Become Familiar with JavaScript

Read the documentation and become familiar with built in objects and features. Become aware, not expert, of what's there. Then make every coding task an opportunity to study closely to see how particular features might help.

Code is also Structure

Like any machine code is constructed as a "depth" structure of parts made of parts made of parts ....

Here is the top of the structure. Intentionally, this top reflects the primary steps of our "main" algorithm - Rubik's Cube Scrambler. typically the UI interaction naturally fit here.

click', function() {
  let scrambleMoves = parseFloat(INPUT.value);
  let scrambleString = '';
  for (let iteration = 0; iteration < scrambleMoves; iteration++) {
    scrambleString += getCurrentScramble();    
  }
  OUTPUT.textContent = scrambleString;
})

Find your Sub-Algorithms and their Discrete Steps

function getCurrentScramble() {
  let doubleTurn = (randomBoolean()) ? '2' : '';
  let currentScramble = getRandomScramble();
  let reverseTurn = (randomBoolean()) ? '\'' : '';  
  return `${doubleTurn}${currentScramble}${reverseTurn} `;  
}

This code makes clear the three parts of a Cube-Side-Label. Each part is calculated separately. The resulting label is more apparent because construction details are encapsulated (hidden away) in functions.

P.S. The ternary operator and string interpolation makes for concise code which helps clarity.


Comments

 let time_lister = 1; // Move the variable declaration outside the function

Comment the WHY, not the "what".

  • If I can't read JS, no comment will help me understand.
  • If I can read JS this comment does not help me understand.
\$\endgroup\$
1
\$\begingroup\$

@radarbob already explained my codepen and please read his explanation before as I not going to cover that part again but going to build upon that covered knowledge!

1 Timer Function

Your timer function is really hard to read and look through as it is not intuitive and rather complex and confusing. Note, that you write code to be readable and maintainable by other developers not to be as short and time-efficient as possible by machines!

To cover the whole part and to make it as efficient as possible in every aspect we have to look into different techniques.

1.1 Double setInterval usage

The biggest issue I have with that entire timer is that you use 2 setInterval functions here. One with a 1000ms and another with a 100ms timer. You should only use 1 setIntervall with the 100ms times as the other will be covered by this as well. So the slower setInterval is just redundant and consumes unnecessary resources.

1.2 Modulus Operator

Another thing you should learn to use is the modulus operator: %. A modulus will call out the remainder of a division:

window.addEventListener('DOMContentLoaded', function() {
  let count = 0;
  setInterval(function() {
    count++;
    console.clear();
    console.log(count % 10);
  }, 200)
})

As said the modulus just gives you the remainder of the division and as such you have a count that can continue and does not have to be reset to 0.

 1 % 10 = (0) remainder: 1
 2 % 10 = (0) remainder: 2
 3 % 10 = (0) remainder: 3
 4 % 10 = (0) remainder: 4
 5 % 10 = (0) remainder: 5
 6 % 10 = (0) remainder: 6
 7 % 10 = (0) remainder: 7
 8 % 10 = (0) remainder: 8
 9 % 10 = (0) remainder: 9
10 % 10 = (1) remainder: 0
11 % 10 = (1) remainder: 1
12 % 10 = (1) remainder: 2
...

1.3 if/else-Statement

I really like that you use an if/else to start and stop the timer function. That is actually useful and appropriate usage.
What I do not like is that you compare with 3 conditions: (tenths === 0 && seconds === 0 && minutes == 0) which takes more than just 3 times as long as comparedwith just one condition.
The smart solution would be to use a boolean variable to track if a timer runs such as timerRuns = false. Then you just need to compare if the timer is not running by using (!timerRuns) which is more time and resource-efficient.
You can take that even a step further and use the ternary conditional operator that was already used with the scramble to let the boolean switch between true and false: timerRuns = (timerRuns) ? false : true;.
A real senior dev will however even take this a step further and toggle a boolean directly by using: timerRuns = !timerRuns which will always call the opposite of a boolean (as long as you expect a boolean to only have 2 states. A boolean technically has 3 states).

timerRuns = !timerRuns

timer is true
timer = not true => false

timer is false
timer = not false => true

1.4 Descrete Functions

Always try to cut down your functions as small as possible and move parts of your function to discrete functions to be more readable. Within your setIntervall you do a lot of stuff that is not needed. Simply iterate the time count in that function and move the displaying or printing of the time to a discrete function such as:

setInterval(function() {
  timer++;
  printFunction();
}, 100);

function printFunction() {
  // all your code to print out the time
}

That I super fast performant and really easy to read. You can even take that a step further and move the printFunction to a dedicated thread (Web Worker) so that you can make use of multi-threading or multi-core processors.

In any case, do not iterate so many variables but rather count the time in deciseconds and then convert those deciseconds to the time you like.

There are different methods to do it. The simplest would be to copy the time to a temporary variable and then subtract the minutes and seconds from it and so on.
My approach takes a little bit more math knowledge but is shorter and slightly (within microseconds) faster.

  1. I get how many minutes have been passed. 1 minute equals 60 seconds or 600 deciseconds. However, I want to have an integer number and not something like 2.3 minutes. This means I round it down (always down) by using Math.floor(): minutes = Math.floor(deciseconds / 600).
  2. I need to count the seconds that have passed but excluding the already subtracted minutes. To subtract the minutes from the deciseconds, I can use the modulus operator deciseconds % 600 which will give me the deciseconds without the subtracted minutes. As I know that 1 second is 10 deciseconds I can then divide that remainder by 10: seconds = Math.floor((decisseconds % 600) / 10).
  3. Last but not least the remaining deciseconds which simply is the last digit that I can get again with the modulus operator by just using: deciseconds = deciseconds % 10

1.5 2-Digit Integers

You created an element containing a 0 that you hide if the seconds are above 9 and show if they are below 10. YOu do that to have a leading zero to always have a 2-digit number. That is inaccessible and quite a nasty way to create such number.
The easier way is to use a zero and append your number (not add) it to the zero as a string. Then you slice the string and keep only the last 2 digits: `('0' + number).slice(-2).

That will works such as this:

Number is 45
  ('0' + 45).slice(-2)
= ('045').slice(-2)
= '45'

Number is 7
  ('0' + 7).slice(-2)
= ('07').slice(-2)
= '07'

Note, that you have a string as a datatype and not a number. So you only shout do that at the very end or have to use parseInt/parseFloat to transform it back as a number.

1.6 innerHTML vs innerText vs textContent

innerHTML is a quite old method. It poses security risks in the form of XSS injections. It also is quite slow as the DOM has to be reparsed. That means, when manipulating the DOM that way it has to be "loaded" again to show it. This is time-consuming and as such slow.

innerText is the same story as innerHTML with the exception that it will only render Text and as such HTML tags as text as well instead of parsing it. But the DOM also needs to be reparsed and as such it is slow but does not poses a security risk.

The faster way to manipulate text without posing a security risk is the usage of textContent. As you only want to update/change text here, you should use this.

1.7 Possible Refactorisation

let time;
let timer;
let timerRuns = false;

document.querySelector('button').addEventListener('click', function() {
  if (!timerRuns) {
    startTimer();
  } else {
    clearInterval(timer);
  }
  
  timerRuns = !timerRuns;
})

function startTimer() {
  time = 0;
  timer = setInterval(function() {  
    time++;
    printTime();
  }, 100);
}

function printTime() {
  let minutes = TwoDigitInt(Math.floor(time / 600));
  let seconds = TwoDigitInt(Math.floor((time % 600) / 10));
  let deciseconds = time % 10;
  console.clear();
  console.log(`${minutes}:${seconds}.${deciseconds}`);
}

function TwoDigitInt(integer) {
  return ((`0${integer}`).slice(-2));
}
<button>Start / Stop</button>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.