ALL POSTS
<p>I ran both versions of the code with the console open and a counter inserted at the jQuery getJSON statement.
</p><p>In both versions, getJSON executed 77 times at Community Central.
</p><p>The original code died after the 77th request
just as expected when dfcontinue exists and gaifrom doesn't. </p><p>I didn't measure the traffic volume. </p>
uncaught exception: TypeError: data['query-continue'].allimages is undefinedjust as expected when dfcontinue exists and gaifrom doesn't. </p><p>I didn't measure the traffic volume. </p>
(Edited by Saftzie)
<p>Well, it seems the code has more bugs than initially thought. It was also developed a long time ago, so a fresh new version is a good thing. But it would be better to hold off on that until the performance can be improved.
</p><p>Edit
</p><p>I think you should just show the first batch of the results, and store the rest. One the requests are done, then simply drop everything else. As far as improving the number of requests, it is not that big of a deal. it is just around 200kb more. Considering that if one uses that scripts frequently it will never become laggy because all duplicates will be deleted every so often.
</p>
(Edited by Dessamator)
<p>I described all these bugs in the opening post.
</p>
- The original code doesn't find all the duplicates
- Some of the duplicates that it does find it lists multiple times
- It errors out on the last iteration
(Edited by Saftzie)
<p>Yes, fixing the bugs is fine. I originally meant that with this script one never really knows when it is done. So if one has a laggy connection it may be a problem.
</p><p>I meant something like this:
</p>
<div dir="ltr" class="mw-geshi mw-content-ltr"><div class="javascript source-javascript">
<span class="kw1">var</span> remainingResults <span class="sy0">=</span> <span class="st0">""</span><span class="sy0">;</span>
<span class="kw1">var</span> isFirst <span class="sy0">=</span> <span class="kw2">true</span><span class="sy0">;</span>
<span class="kw1">if</span> <span class="br0">(</span>output<span class="br0">)</span> <span class="br0">{</span>
<span class="kw1">if</span> <span class="br0">(</span>isFirst<span class="br0">)</span> <span class="br0">{</span>
$<span class="br0">(</span><span class="st0">'#mw-dupimages'</span><span class="br0">)</span>.<span class="me1">append</span><span class="br0">(</span>output <span class="sy0">+</span> <span class="st0">'</ul>'</span><span class="br0">)</span><span class="sy0">;</span>
isFirst <span class="sy0">=</span> <span class="kw2">false</span><span class="sy0">;</span>
<span class="br0">}</span>
<span class="kw1">else</span><span class="br0">{</span>
output <span class="sy0">+=</span> <span class="st0">"more output"</span>
<span class="br0">}</span>
<span class="br0">}</span>
<span class="co1">// do other stuff</span>
<span class="co1">// when processing ends</span>
<span class="kw1">if</span> <span class="br0">(</span>output<span class="br0">)</span> <span class="br0">{</span>
$<span class="br0">(</span><span class="st0">'#mw-dupimages'</span><span class="br0">)</span>.<span class="me1">append</span><span class="br0">(</span>output <span class="sy0">+</span> <span class="st0">'</ul>'</span><span class="br0">)</span><span class="sy0">;</span>
<span class="br0">}</span></div></div>
<p>Maybe adding some kind of indicator that stuff is still happening in the background would be nice too.
</p><p>It is a minor change, so I don't think people will mind.
</p>
(Edited by Dessamator)
<p>There is the spinner on the right, though it's not particularly noticeable to me. We could move it to the left, I suppose.
</p><p>Amusingly there are talk page comments from a while ago about how the spinner kept spinning after the code exited. That was probably because the code died instead of exiting normally, but the fix at the time was to move hiding the spinner to before the point where the error caused the code to die.
</p>
(Edited by Saftzie)
<p>Something like this is what I meant, although it's not really a bug.
</p><p>
</p>
</p>
</p>
</p>
- Edit
$<span class="br0">(</span><span class="st0">'#dupImagesProgress'</span><span class="br0">)</span>.<span class="me1">show</span><span class="br0">(</span><span class="br0">)</span><span class="sy0">;</span>
<span class="kw1">var</span> indicator <span class="sy0">=</span> stylepath <span class="sy0">+</span> <span class="st0">'/common/progress-wheel.gif'</span><span class="sy0">,</span>
url <span class="sy0">=</span> <span class="st0">'/api.php?format=json&action=query&prop=duplicatefiles&dflimit=500&generator=allimages&gailimit=500'</span><span class="sy0">;</span>
dil <span class="sy0">=</span> dil <span class="sy0">||</span> <span class="br0">{</span><span class="br0">}</span><span class="sy0">;</span>
<span class="kw1">if</span> <span class="br0">(</span><span class="sy0">!</span><span class="br0">(</span>$<span class="br0">(</span><span class="st0">'#dupImagesProgress'</span><span class="br0">)</span>.<span class="me1">length</span><span class="br0">)</span><span class="br0">)</span> <span class="br0">{</span>
$<span class="br0">(</span><span class="st0">'#mw-dupimages'</span><span class="br0">)</span>.<span class="me1">prepend</span><span class="br0">(</span><span class="st0">'<span style="float: right;" id="dupImagesProgress" title="In progress..."><img src="'</span> <span class="sy0">+</span> indicator <span class="sy0">+</span> <span class="st0">'" style="vertical-align: baseline;" border="0" alt="In progress..." /></span>'</span><span class="br0">)</span><span class="sy0">;</span>
<span class="br0">}</span></div></div>
<p>change to
</p>
<div dir="ltr" class="mw-geshi mw-content-ltr"><div class="javascript source-javascript"> <span class="kw1">var</span> indicator <span class="sy0">=</span> stylepath <span class="sy0">+</span> <span class="st0">'/common/progress-wheel.gif'</span><span class="sy0">,</span>
url <span class="sy0">=</span> <span class="st0">'/api.php?format=json&action=query&prop=duplicatefiles&dflimit=500&generator=allimages&gailimit=500'</span><span class="sy0">;</span>
dil <span class="sy0">=</span> dil <span class="sy0">||</span> <span class="br0">{</span><span class="br0">}</span><span class="sy0">;</span>
<span class="kw1">if</span> <span class="br0">(</span><span class="sy0">!</span><span class="br0">(</span>$<span class="br0">(</span><span class="st0">'#dupImagesProgress'</span><span class="br0">)</span>.<span class="me1">length</span><span class="br0">)</span><span class="br0">)</span> <span class="br0">{</span>
$<span class="br0">(</span><span class="st0">'#mw-dupimages'</span><span class="br0">)</span>.<span class="me1">prepend</span><span class="br0">(</span><span class="st0">'<div id="dupImagesProgress" style="height: 0; text-align: center;"><img src="'</span> <span class="sy0">+</span> indicator <span class="sy0">+</span> <span class="st0">'" style="border: 0 none;" alt="In progress..." /></div>'</span><span class="br0">)</span><span class="sy0">;</span>
<span class="br0">}</span></div></div>
<p></p>
- Remove from the middle of the code
$<span class="br0">(</span><span class="st0">'#dupImagesProgress'</span><span class="br0">)</span>.<span class="me1">hide</span><span class="br0">(</span><span class="br0">)</span><span class="sy0">;</span></div></div> <p>
</p>
- Edit
<span class="kw1">else</span> <span class="br0">{</span>
<span class="kw1">if</span> <span class="br0">(</span>output<span class="br0">)</span> <span class="br0">{</span>
$<span class="br0">(</span><span class="st0">'#mw-dupimages'</span><span class="br0">)</span>.<span class="me1">append</span><span class="br0">(</span>output <span class="sy0">+</span> <span class="st0">'</ul>'</span><span class="br0">)</span><span class="sy0">;</span>
<span class="br0">}</span>
<span class="br0">}</span></div></div>
<p>change to
</p>
<div dir="ltr" class="mw-geshi mw-content-ltr"><div class="javascript source-javascript"> <span class="kw1">else</span> <span class="br0">{</span>
$<span class="br0">(</span><span class="st0">'#dupImagesProgress'</span><span class="br0">)</span>.<span class="me1">empty</span><span class="br0">(</span><span class="br0">)</span><span class="sy0">;</span>
<span class="kw1">if</span> <span class="br0">(</span>output<span class="br0">)</span> <span class="br0">{</span>
$<span class="br0">(</span><span class="st0">'#mw-dupimages'</span><span class="br0">)</span>.<span class="me1">append</span><span class="br0">(</span>output <span class="sy0">+</span> <span class="st0">'</ul>'</span><span class="br0">)</span><span class="sy0">;</span>
<span class="br0">}</span>
<span class="br0">}</span></div></div>
(Edited by Saftzie)
<p>Oh okay, that should work too.
</p>
(Edited by Dessamator)
<p>A slightly better idea, I think, is to move the spinner to the middle of the top line, instead of the left, and set its layout height to 0.
</p>
<div dir="ltr" class="mw-geshi mw-content-ltr"><div class="javascript source-javascript">
I'll make changes to the opening post and my sandbox incorporating the above edits. </p>
$<span class="br0">(</span><span class="st0">'#mw-dupimages'</span><span class="br0">)</span>.<span class="me1">prepend</span><span class="br0">(</span><span class="st0">'<div id="dupImagesProgress" style="height: 0; text-align: center;"><img src="'</span> <span class="sy0">+</span> indicator <span class="sy0">+</span> <span class="st0">'" style="border: 0 none;" alt="In progress..." /></div>'</span><span class="br0">)</span><span class="sy0">;</span></div></div> <p>
I'll make changes to the opening post and my sandbox incorporating the above edits. </p>
(Edited by Saftzie)
<p>I can't see where the current code is right now, is there a link I'm missing? As far as the code style goes, I'd be inclined to use the mediawiki.api module for the API requests as it's more future proof than working with the URL strings directly (in case someone wants to port it to wikipedia, etc.). It also saves having to remember to explicitly set the format parameter (which I'm always forgetting). Another MediaWiki specific idea is to use mw.config over accessing the global directly. I can never tell if Wikia rely on these being globals due to the complexity of how they handle globals in general, but it's probably a good idea.
</p><p>As far as performance goes, obviously more requests are going to have side effects, but as requests are typically run async in JS is this that much of a performance hit? Numbers don't mean an awful lot without timing as well.
</p>
(Edited by Cqm)
<p>I incorporated the changes into the live version a few weeks ago. I intended for this review just to fix bugs, because the previous version was very buggy. The previous version is the link in the original post.
</p><p>I'd argue the updated code performs better, since it returns more complete result sets. However, it can take more time (and possibly requests) to run on wikis that have a lot of duplicates, since the previous version didn't return everything. Not returning everything can be accomplished fastest by not running at all, eh?
</p>
(Edited by Saftzie)