Fix issue#1 #27
Fix issue#1 #27
Conversation
| describe('testing deck.ts', () => { | ||
| test('makeCard', () => { | ||
| const card = makeCard('number', 'blue', '3'); | ||
| expect(card.color).toBe('blue'); | ||
| }); | ||
| test('getShuffledDeck', () => { | ||
| const deck = getShuffledCardDeck(); | ||
| console.log(deck); |
There was a problem hiding this comment.
The purpose of tests is not to just console log things. That might have been required during development, but here, you should test whether the card deck correctly creates all the required cards. For example, you should expect there to be 25 cards per color, and 19 of them should be number cards, etc..
There was a problem hiding this comment.
Yes i have checked them all like each color cards, special cards , and length of deck to be 108
There was a problem hiding this comment.
What I meant was for you to write a test which counts the number of red green etc and assert that they are 25
Then count the no. of wildcards and assert that there are 8, etc.
Writing tests ensures that future changes do not break existing functionality.
backend/uno-game-engine/deck.ts
Outdated
| The function makeCard is used to make the card objects. | ||
| @returns {Array} deck - An array of 108 UNO cards. | ||
| */ | ||
| export default function getShuffledCardDeck(): Array<UNOCard> { |
There was a problem hiding this comment.
Can you remove default from here?
| deck.push(makeCard('number', col, val)); | ||
| if (val === '0') break; | ||
| } | ||
| } else { |
There was a problem hiding this comment.
Great job on the algorithm!
There was a problem hiding this comment.
Suggested some changes. Focus on improving the tests. Look up jest testing library.
Also the commit title of a preparatory commit doesn't literally have to say so. The commit message should be the same as any other commit, whose format is mentioned in the docs. (just that the Fixes clause is omitted, as that commit doesn't actually fix the issue)
Side note: You can use github's request review feature once you are done with the changes.
Additionally, the PR description can be a bit more descriptive.
Abhishek-Punhani
force-pushed
the
FixIssue#1
branch
4 times, most recently
from
e840792 to
104c0bb
Compare
Abhishek-Punhani
force-pushed
the
FixIssue#1
branch
from
704966f to
44927c8
Compare
|
@Abhishek-Punhani Since this is a high priority issue, I'll have to go with #30 if the requested changes are not made by today evening. |
Abhishek-Punhani
force-pushed
the
FixIssue#1
branch
16 times, most recently
from
2051ac9 to
a3ac3d0
Compare
Added wild color for wild cards like colchange and draw4 cards these changes are made in types.d.ts
Abhishek-Punhani
force-pushed
the
FixIssue#1
branch
from
a3ac3d0 to
9cc34c4
Compare
This commit involves one preperatory commit : adding wild color to CardColors Added deck generation function to initialize the deck according to given constraints The function generates cards and push them the deck array generating cards of each color and also wild cards Also have checked the deck composition with jest tests Fixes :shivansh-bhatnagar18#1
This commit involves one preperatory commit : adding wild color to CardColors Added deck generation function to initialize the deck according to given constraints The function generates cards and push them the deck array generating cards of each color and also wild cards Also have checked the deck composition with jest tests Fixes :shivansh-bhatnagar18#1
backend/deck.js
Fixed the function to initialize the deck according to the requirements and added valid checks to check the composition of deck
Fixes: #1