no-stack-dub-sack / cs-playground-react

In-Browser Algorithm and Data Structures Practice
http://cs-playground-react.surge.sh/
MIT License
520 stars 91 forks source link

fix loop-protect issue, change timeout length #59

Closed no-stack-dub-sack closed 6 years ago

mansisce commented 6 years ago

Please provide more details on the same

no-stack-dub-sack commented 6 years ago

@mansisce right now there is an issue with the loop protection, for example, if you try to run the solution of Anagram Palindrome, the loop-protect babel transform is not working correctly and the tests don't run. This has to do with the fact that there was an issue with adding loop-protect to code that is unaffected by the babel transform (i.e. code that doesn't have loops, or loops that aren't recognized by loop-protect as having the potential to be infinite like for...of and for...in.

I've figured out the issue though, and essentially what is happening is that when loop-protect transforms code, it trims white space at either end of the code string, and thus is concatenating it with the tests array undesirably.

So, what needs to happen, is starting here, lines 30 & 31 can be replaced with:

  // apply loop-protect if not disabled by user
  if (!DISABLE_LOOP_PROTECT.test(code)) 
    code = loopProtect(code) + '\n'

this will make it so that all code is transformed by loop-protect unless the user has left a // DISABLE LOOP PROTECT comment in their code.

Since we're not using HAS_LOOPS any more, or trimComments both of those imports can be removed, as well as the trim-code.js file and the HAS_LOOPS declaration in the regexp.js file.

You can test that all of this works by running the anagram palindrome solution to make sure all the tests pass, and by going to the Sorting Algo Benchmarks repl and making sure that runs as well (toggle on and off the disable loop protect comment and make sure it runs both ways, when disable loop protect is enabled, it should throw a range error)

Let me know if all of this makes sense or if you have any questions!

no-stack-dub-sack commented 6 years ago

Also, if you're working in these files, starting here, lines 41-44 can be replaced with console.log(e.toString()) as long as we change the error being thrown to a new RangeError instead of just new Error right here

no-stack-dub-sack commented 6 years ago

Finally, I've also been thinking about changing the timeout from 500 to 200 ms for loop-protection. If you end up taking this on, let's go ahead and make that change as well (to both the timeout declaration and the error message in the loop-protect.js file