Suggested improvement #1: Rationalise Custom parameters
Closed this issue · 0 comments
Hi Michael, since MineSweeper was dropped from Windows I was looking for a close replacement and I came across this one of yours which is probably slightly over-engineered but has introduced me to Workers and other advanced features in JavaScript that I didn't even know were there which makes it by far my favourite implementation. While looking at the code I did find some areas I thought might stand a little improvement, nothing fundamental you understand but just to bring it closer to the original version. I will add each as a separate issue so we can tick them off (as I intend to attempt to make the changes myself in a fork as you suggested in our email exchange), so let's start with this:
The Custom parameters are not validated nor rationalised so you can in fact specify a grid that is too large for Java to handle (e.g. 100 x 100 with 9000 mines) causing the code to "hang" when you click the first square. The original version limited the dimensions to be:
x: between 9 & 30 (I suggest just implementing a maximum of 30, so a minimum of 1)
y: between 9 & 24 (ditto)
mines: the original performs a weird, non-linear calculation to determine a maximum, I suggest a maximum of (number of squares-1) so there is at least one square they can click on
I suggest the Minesweeper code itself performs this rationalisation in the js file to protect the logic, then the same (or more strict) limits can also be applied in the outer code giving a better user experience.