patternjavascriptMinor
Show number of pages next to Flickr threads
Viewed 0 times
shownumberthreadsnextpagesflickr
Problem
This is a userscript that simply adds the number of pages that a thread has next to its title for Flickr interface. (FWIW it's not really done because the script is only loaded at page load, but not reloaded on subsequent AJAX calls that the user may make).
I'm far from being a seasoned JS developer, so I'm looking for guidelines to potentially bring the script to state-of-the-art JS standards.
Example of questions that I'm not sure about:
Any pointers are appreciated! (this is also my first userscript, so feedback about the userscript itself is also appreciated)
```
// ==UserScript==
// @name Flickr - Number of pages in a thread
// @namespace flickr_
// @description This user script is simply displaying the number of pages in a thread on the main page of the group
// @include http://flickr.com/groups/*
// @version 1
// @grant none
// @run-at document-start
// @require http://ajax.googleapis.com/ajax/libs/jquery/1.2.6/jquery.js
// ==/UserScript==
var api_key = 'fe753c0dd6068fb6c74cb70458b1b9a0';
var POSTS_PER_PAGE = 100;
function call_flickr(method, params, callback) {
'use strict';
var url = "https://api.flickr.com/services/rest?";
params.api_key = api_key;
params.format = 'json';
params.nojsoncallback = 1;
params.method = method;
$.get(url, params, function (data) {
callback(JSON.parse(data));
});
}
function number_of_pages(data) {
'use strict';
var url = '/groups/' + data.topics.path_alias + '/discuss/';
data.topics.topic.forEach(function (topic) {
var pages_number = Math.ceil(topic.count_replies / POSTS_PER_PAGE);
var link = url + topic.id + '/';
var last_link = link + 'page' + pages_numb
I'm far from being a seasoned JS developer, so I'm looking for guidelines to potentially bring the script to state-of-the-art JS standards.
Example of questions that I'm not sure about:
- Should I use promises?
- Is the workflow done well?
- Is the inline CSS/HTML evil in this case?
- Should I omit jQuery?
- Am I making too many jQuery calls? (i.e. should I just make one query and find the next rows through
siblingsor something similar?)
Any pointers are appreciated! (this is also my first userscript, so feedback about the userscript itself is also appreciated)
```
// ==UserScript==
// @name Flickr - Number of pages in a thread
// @namespace flickr_
// @description This user script is simply displaying the number of pages in a thread on the main page of the group
// @include http://flickr.com/groups/*
// @version 1
// @grant none
// @run-at document-start
// @require http://ajax.googleapis.com/ajax/libs/jquery/1.2.6/jquery.js
// ==/UserScript==
var api_key = 'fe753c0dd6068fb6c74cb70458b1b9a0';
var POSTS_PER_PAGE = 100;
function call_flickr(method, params, callback) {
'use strict';
var url = "https://api.flickr.com/services/rest?";
params.api_key = api_key;
params.format = 'json';
params.nojsoncallback = 1;
params.method = method;
$.get(url, params, function (data) {
callback(JSON.parse(data));
});
}
function number_of_pages(data) {
'use strict';
var url = '/groups/' + data.topics.path_alias + '/discuss/';
data.topics.topic.forEach(function (topic) {
var pages_number = Math.ceil(topic.count_replies / POSTS_PER_PAGE);
var link = url + topic.id + '/';
var last_link = link + 'page' + pages_numb
Solution
First at all I not so used to
I can point:
Regarding your questions:
Few changes I would made:
Only declare one,
Whit this change I don't think that you are gaining so much whit your current function
So I would split it in two functions each one whit the call. Like this:
usercript. But in general I think your solution is overengineered.I can point:
- Readability of code is not good.
- The "standar" for JavaScript usually is camel case.
Regarding your questions:
- You don't have any complicated workflow or expecting several answers, so I wouldn't use promises.
- As I said before, I think your code is really hard to read for only being three functions
- You could remove the HTML in line creating the elements by JavaScript, but I don't think is so much for calling it "evil"...
- You could
- Don't think so
Few changes I would made:
Only declare one,
'use strict'var url = "https://api.flickr.com/services/rest?";
var POSTS_PER_PAGE = 100;
var params = {
url: document.URL,
api_key: 'fe753c0dd6068fb6c74cb70458b1b9a0',
method: 'flickr.urls.lookupGroup',
format: 'json',
nojsoncallback: 1
};Whit this change I don't think that you are gaining so much whit your current function
call_flickr. So I would split it in two functions each one whit the call. Like this:
'use strict';
var url = "https://api.flickr.com/services/rest?";
var POSTS_PER_PAGE = 100;
var params = {
url: document.URL,
api_key: 'fe753c0dd6068fb6c74cb70458b1b9a0',
format: 'json',
nojsoncallback: 1
};
function numberOfPages(data) {
var url = '/groups/' + data.topics.path_alias + '/discuss/';
data.topics.topic.forEach(function (topic) {
var pages_number = Math.ceil(topic.count_replies / POSTS_PER_PAGE);
var link = url + topic.id + '/';
var last_link = link + 'page' + pages_number;
var current_element = $('a[href=' + link + ']');
if (pages_number > 1) {
current_element.after(' ' + pages_number + '');
}
});
}
function threadsList(data) {
params.group_id = data.group.id;
params.method = 'flickr.groups.discuss.topics.getList';
$.get(url, params, function (data) {
numberOfPages(JSON.parse(data));
});
}
function main() {
params.method = 'flickr.urls.lookupGroup';
$.get(url, params, function (data) {
threadsList(JSON.parse(data));
});
}
main();Code Snippets
var url = "https://api.flickr.com/services/rest?";
var POSTS_PER_PAGE = 100;
var params = {
url: document.URL,
api_key: 'fe753c0dd6068fb6c74cb70458b1b9a0',
method: 'flickr.urls.lookupGroup',
format: 'json',
nojsoncallback: 1
};'use strict';
var url = "https://api.flickr.com/services/rest?";
var POSTS_PER_PAGE = 100;
var params = {
url: document.URL,
api_key: 'fe753c0dd6068fb6c74cb70458b1b9a0',
format: 'json',
nojsoncallback: 1
};
function numberOfPages(data) {
var url = '/groups/' + data.topics.path_alias + '/discuss/';
data.topics.topic.forEach(function (topic) {
var pages_number = Math.ceil(topic.count_replies / POSTS_PER_PAGE);
var link = url + topic.id + '/';
var last_link = link + 'page' + pages_number;
var current_element = $('a[href=' + link + ']');
if (pages_number > 1) {
current_element.after(' <a style="font-size:12px; color:#aaa;" href="' +
last_link + '">' + pages_number + '</a>');
}
});
}
function threadsList(data) {
params.group_id = data.group.id;
params.method = 'flickr.groups.discuss.topics.getList';
$.get(url, params, function (data) {
numberOfPages(JSON.parse(data));
});
}
function main() {
params.method = 'flickr.urls.lookupGroup';
$.get(url, params, function (data) {
threadsList(JSON.parse(data));
});
}
main();Context
StackExchange Code Review Q#93965, answer score: 2
Revisions (0)
No revisions yet.