Conversation
dev-lovedeep
left a comment
dev-lovedeep
left a comment
There was a problem hiding this comment.
make these changes then i will merge your PR
|
|
||
|
|
||
| // To view all cards | ||
| function viewAllCards(){ |
There was a problem hiding this comment.
showAllCards would be a better name
| // To view all cards | ||
| function viewAllCards(){ | ||
| // Cards are made viewable when memoryCounter is greater than 0 | ||
| if(memoryCounter>0){ |
There was a problem hiding this comment.
this check should not be inside the function. To make a function reusable, it should do what its name suggests. Checks should be placed where the function is called.
for ex: if we add a hint button which show the all cards , then we can reuse this function, but if this check is there we cannot reuse it
| const interval = setInterval(function(){ | ||
| if(memoryCounter>0){ // Makes a check on memoryCounter and stops after 5 secs | ||
| memoryCounter--; | ||
| viewAllCards(); |
There was a problem hiding this comment.
this viewAllCards function is called every time until the memory counter is zero, which is not good for performance. Add some conditions so that it do not run unnecessarily
|
@kunal2812 are you working on this issue? |
Sorry sir I was out for a while and didn't have access to my pc co could not make the required changes. |
dev-lovedeep
left a comment
dev-lovedeep
left a comment
There was a problem hiding this comment.
need to work on your variable naming , otherwise code was ok
2 participants
Fixes #8