Module #3 (Tahsina Bintay Azam)#35
Conversation
src/hooks/snake.js
Outdated
|
|
||
| useInterval(runSingleStep, 200); | ||
| useInterval(addFood, 3000); | ||
| useInterval(addObject("food"), 3000); |
There was a problem hiding this comment.
useInterval(()=>addObject("food"), 3000);
src/hooks/snake.js
Outdated
| useInterval(addObject("food"), 3000); | ||
| useInterval(removeFood, 100); | ||
| useInterval(addPoison, 15000); | ||
| useInterval(addObject("poison"), 15000); |
There was a problem hiding this comment.
useInterval(()=>addObject("poison"), 3000);|
@MahdiMurshed thanks a lot, I did what you suggested and my code works just fine <3 |
src/hooks/snake.js
Outdated
| setObjects([]); | ||
| }, []); | ||
|
|
||
| const isFood = useCallback( |
There was a problem hiding this comment.
isFood and isPoison are basicallly same.Can't we make one function to check both ? like
isObjectOfType(type)There was a problem hiding this comment.
Yes added that :) thanks for pointing it out <3
src/hooks/snake.js
Outdated
| //suggested addObject method for food and poison ----------------> | ||
| const addObject = useCallback((type) => { | ||
| let newObject = getRandomCell(type); | ||
| while (isSnake(newObject) || isFood(newObject)) { |
src/hooks/snake.js
Outdated
| // } | ||
| }, []); | ||
| //removeObject method for removing food or poison--------------> | ||
| const removeObject = useCallback((type) => { |
There was a problem hiding this comment.
we are deleting objects whose age are greater than 10 sec .So why do we need to check object type while deleting any object?
There was a problem hiding this comment.
yes, you were right, for some reason that part of the code was causing the problem, I thought I should have the control over when I wanted to remove my poison cell that's why I was checking object Type. Thanks btw.
src/hooks/snake.js
Outdated
| setFoods((currentFoods) => | ||
| currentFoods.filter((food) => Date.now() - food.createdAt <= 10 * 1000) | ||
| setObjects((currentFoods) => | ||
| currentFoods.filter( |
There was a problem hiding this comment.
we're removing an element whether it is food or poison for the same time-lapse. We don't need to check the type here.
There was a problem hiding this comment.
yes, I totally agree. For some reason that part of the code was causing the problem, I thought I should have the control over when I wanted to remove my poison cell that's why I was checking object Type. Thanks btw.
src/hooks/snake.js
Outdated
| setObjects((currentFoods) => | ||
| currentFoods.filter( | ||
| (food) => | ||
| Date.now() - food.createdAt <= 10 * 1000 && food.type === "food" |
There was a problem hiding this comment.
We can use the CellType here instead of the string.
food.type===CellType.Food
src/hooks/snake.js
Outdated
| useInterval(() => addObject("food"), 3000); | ||
| useInterval(() => removeObject("food"), 100); | ||
| useInterval(() => addObject("poison"), 15000); | ||
| useInterval(() => removeObject("poison"), 100); |
There was a problem hiding this comment.
we dont need to call removeObject twice and removeObject doesn't need to take any parameter
src/hooks/snake.js
Outdated
| useInterval(runSingleStep, 200); | ||
| useInterval(() => addObject("food"), 3000); | ||
| useInterval(() => removeObject("food"), 100); | ||
| useInterval(() => addObject("poison"), 15000); |
There was a problem hiding this comment.
You are not taking the advantage of CellType object .By hard coding "food" you are creating a source of bug . Do It instead
addObject(CellType.Food)
removeObject(CellType.Poison)
src/hooks/snake.js
Outdated
| useInterval(() => addObject(CellType.Food), 3000); | ||
| useInterval(() => removeObject(CellType.Food), 100); | ||
| useInterval(() => addObject(CellType.Poison), 15000); | ||
| useInterval(() => removeObject(CellType.Poison), 100); |
There was a problem hiding this comment.
Sorry,removeFood doesn't need any parameter, my bad.And why calling twice the same function at same time interval? you are not removing the object depending on types .You are removing objects whose age is greater than 10 sec regardless of the type.
useInterval(() => addObject(CellType.Food), 3000);
useInterval(() => addObject(CellType.Poison), 15000);
useInterval(removeObject, 100);There was a problem hiding this comment.
dang , missed that part :")
| isSnake(newObject) || | ||
| isObjectOfType(newObject, CellType.Food) || | ||
| isObjectOfType(newObject, CellType.Poison) | ||
| ) { |
There was a problem hiding this comment.
as you previously forgot to add isPoison() in while condition, There is a possibility that you will forget it again. So why not create a separate function named isObject whose job is to check whether the given coordinate is a food or poison .
tried refactoring and tried to merge food and poison into the same state.