patternjavascriptMinor
YouTube-to-Spotify converter
Viewed 0 times
youtubespotifyconverter
Problem
I have been doing some JS lately and I would like to get some constructive criticism.
The project I am working on
``
.replace(/ /g, ' ')
.replace(/\[.*\]/g, '')
.replace(/lyrics/g, '')
.replace(/\(.*\)/g, '')
.replace(/\.\*/g, '')
.replace(/ /g, ' ');
return newTitle;
}
func
The project I am working on
``
var SpotifiedList = [],
youtubeSongs, spotifySongs = 0,
spotifiedSongs = 0,
playNext;
$(document).ready(function () {
var id = window.location.hash;
if (id) {
$("#playlistID").val(id.substring(1));
startProcess();
}
});
function resetTimer() {
console.log("timer reset!")
clearTimeout(playNext);
}
function startProcess() {
$("#errorOccured").hide();
if ($("#playlistID").val().length playlist ID.");
} else {
window.location.hash = $("#playlistID").val();
$("#list").html('This might take some time.');
$(".alert-info").html("Loading");
$(".alert-danger").hide();
retrieveYoutubePlaylist($("#playlistID").val(), 1, [], 1);
}
}
function retrieveYoutubePlaylist(id, startIndex, list, k) {
$("#loading").show();
$.ajax({
url: 'https://gdata.youtube.com/feeds/api/playlists/' + id + '?v=2&alt=json&start-index=' + startIndex + '&max-results=50',
dataType: 'json',
success: function (data) {
if (typeof data.feed.entry === 'undefined') {} else {
for (var i = 0; i 0) {
spotifiedSongs += 1;
$("#list").append('' + data.tracks[0].artists[0].name + ' - ' + data.tracks[0].name + '');
var info = {
'url': data.tracks[0].href,
'length': Math.round(data.tracks[0].length * 1000),
'name': data.tracks[0].artists[0].name + ' - ' + data.tracks[0].name
}
SpotifiedList.push(info);
}
if (youtubeSongs == spotifySongs) {
$(".alert-info").html('\
\
Play/Skip\
\
Converted songs: ' + spotifiedSongs + '/' + youtubeSongs);
}
});
}
function parseTitle(title) {
var newTitle = title
.replace(/[-!$%^&_+|~={}:";'<>?,.\/]/g, '').replace(/ /g, ' ')
.replace(/\[.*\]/g, '')
.replace(/lyrics/g, '')
.replace(/\(.*\)/g, '')
.replace(/\.\*/g, '')
.replace(/ /g, ' ');
return newTitle;
}
func
Solution
General
I could work with this code, I only have a few remarks.
Globals
You should consider as a minimum to have your 4 globals in an object.
You could even add most of the functions to the player object
resetTimer
You should remove the
startProcess
You could consider caching the jQuery queries, maybe in the before proposed player object.
retrieveYoutubePlaylist
retrieveSpotify
Encoding
Building HTML should be contained to separate functions.
parseTitle
I have no idea what that does, a few comments are needed there
play
I am curious, what does
I could work with this code, I only have a few remarks.
Globals
You should consider as a minimum to have your 4 globals in an object.
var player =
{
spotifiedList : [],
youtubeSongs : 0,
spotifySongs : 0,
spotifiedSongs : 0,
playNext : 0
}You could even add most of the functions to the player object
resetTimer
You should remove the
console.log, which makes this a simple one liner. If you keep it ( function name neatly explains what it does ), make sure to call it from play() instead of doing a straight clearTimeout(playNext).startProcess
You could consider caching the jQuery queries, maybe in the before proposed player object.
retrieveYoutubePlaylist
if (typeof data.feed.entry === 'undefined') {} else { is just wrong.retrieveSpotify
Encoding
onclick= is wrong as well. Building HTML should be contained to separate functions.
parseTitle
I have no idea what that does, a few comments are needed there
play
I am curious, what does
>> 0 get you ? Force it into an integer?Code Snippets
var player =
{
spotifiedList : [],
youtubeSongs : 0,
spotifySongs : 0,
spotifiedSongs : 0,
playNext : 0
}Context
StackExchange Code Review Q#30580, answer score: 2
Revisions (0)
No revisions yet.