Skip to content

Roee kazma branch#1

Open
RoeeKazma wants to merge 16 commits into
masterfrom
RoeeKazmaBranch
Open

Roee kazma branch#1
RoeeKazma wants to merge 16 commits into
masterfrom
RoeeKazmaBranch

Conversation

@RoeeKazma
Copy link
Copy Markdown
Owner

No description provided.

Comment thread index.js

function playSong(id) {
// your code here
let songPlayer = findSongById(id);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not const?

Comment thread index.js
// your code here
if (findSongById(id)===undefined)
{
throw "This is not a valid ID"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new Error?

Comment thread index.js

const addedSong = // making a new song to push to the array
{
id: id,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be replaced by {id,title,album,artist,duration: reverseDurationConvertor(duration)}

Comment thread index.js
};

player.playlists.push(newPlaylist);
console.log(newPlaylist);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not necessary

Comment thread index.js
function editPlaylist(playlistId, songId) {

let currentPlaylist = findPlaylistById(playlistId);
let currentSong= isSongInPlaylist(songId, playlistId)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants