freeCodeCamp icon indicating copy to clipboard operation
freeCodeCamp copied to clipboard

fix grammar issues and update tests in step 38 of music player project

Open jdwilkin4 opened this issue 1 year ago • 4 comments

Describe the Issue

There are a few grammar issues in this description. Plus the description can be broken up with spacing a little bit better for readability.

updated description

Before playing the song, you need to make sure it starts from the beginning. This can be achieved by the use of the `currentTime` property on the `audio` object. 

Add an `if` statement to check whether the `userData?.currentSong` is falsy *OR* if `userData?.currentSong.id` is strictly not equal to `song.id`. This condition will check if no current song is playing or if the current song is different from the one that is about to be played.

Inside your `if` block, set the `audio.currentTime` property to `0`.

Also the tests are looking for a specific answer of userData?.currentSong === null || userData?.currentSong.id !== song.id for the condition. But I don't see why we need to specifically check for userData?.currentSong === null when either of these options would be valid

!userData?.currentSong || userData?.currentSong.id !== song.id

userData?.currentSong === null || userData?.currentSong.id !== song.id

userData?.currentSong === undefined || userData?.currentSong.id !== song.id

The tests should be more flexible to allow any valid answer.

Also, order shouldn't matter here because we want to test that campers understand the concept. Not forcing a particular order here.

This should pass but currently doesn't

if(userData?.currentSong.id !== song.id||userData?.currentSong === null)

Affected Page

https://www.freecodecamp.org/learn/javascript-algorithms-and-data-structures-v8/learn-basic-string-and-array-methods-by-building-a-music-player/step-38

Your code

see explanation above

Expected behavior

see explanation above

Screenshots

No response

System

  • Device: [e.g. iPhone 6, Laptop]
  • OS: [e.g. iOS 14, Windows 10, Ubuntu 20.04]
  • Browser: [e.g. Chrome, Safari]
  • Version: [e.g. 22]

Additional context

No response

jdwilkin4 avatar Feb 14 '24 04:02 jdwilkin4

Also, order shouldn't matter here because we want to test that campers understand the concept. Not forcing a particular order here.

This should pass but currently doesn't

if(userData?.currentSong.id !== song.id||userData?.currentSong === null)

There's slight issue with this. If the concept in this step is using the ||, in a way where order doesn't matter, this is not the best example.

userData?.currentSong.id !== song.id||userData?.currentSong === null
  • Second part is obsolete, there's no way that userData?.currentSong === null could be true when it is reached in this order. It will always be false.
    • If userData?.currentSong.id !== song.id is true, then second part is skipped by definition.
    • If userData?.currentSong.id !== song.id is false, because it didn't throw (see below) then userData.currentSong.id must exists, therefore userData?.currentSong === null cannot be true
userData?.currentSong === null || userData?.currentSong.id !== song.id
  • It can throw error. The point of using || in the condition above (where second condition depends on the first one) is that the second part is checked only when first part is false. In this case it means that userData.currentSong is not null, so it is possible to check what is the value of userData.currentSong.id. Otherwise, when userData.currentSong is null, userData?.currentSong.id will throw error:
Uncaught TypeError: Cannot read properties of null (reading 'id')
  • What about userData?.currentSong?.id !== song.id||userData?.currentSong === null? (notice the second question mark added in the first part). Well... the second condition is then again obsolete.

gikf avatar Mar 04 '24 21:03 gikf

@gikf

If order does matter then the directions need to specify that. Otherwise, campers will not understand why this should be allowed to pass

if(userData?.currentSong.id !== song.id||userData?.currentSong === null)

either an update needs to be to the directions on hint text. Or both.

jdwilkin4 avatar Mar 04 '24 21:03 jdwilkin4

Add an if statement to check whether the userData?.currentSong is falsy OR if userData?.currentSong.id is strictly not equal to song.id. This condition will check if no current song is playing or if the current song is different from the one that is about to be played. Despite of using the logical OR, order in this statement is important, as userData?.currentSong.id depends on the existence of userData?.currentSong.

What do you think?

gikf avatar Mar 07 '24 20:03 gikf

I like it.

I would just change the first part to this

Despite the use of the logical _OR_ operator,

jdwilkin4 avatar Mar 07 '24 21:03 jdwilkin4