Created
December 5, 2014 21:53
-
-
Save m5m1th/a34843e034716cb24af8 to your computer and use it in GitHub Desktop.
Dev Challenge! What are all the issues with this code? Hint: there are several.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
var hyperquest = require('hyperquest'); | |
var result; | |
module.exports = function downloadUtf8FileIntoString(url, cb) { | |
if (!url) return cb(Error('url is required')); | |
result = ''; | |
hyperquest(url) | |
.pipe(function (chunk, enc, tCb) { | |
result += chunk.toString('utf8'); // binary to utf8 | |
tCb(); | |
}) | |
.on('finish', function () { | |
cb(result); // all done | |
}) | |
} |
- Isn't true for all streams: see: http://nodejs.org/api/stream.html#stream_event_finish
You're right -- updated to be more specific
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Answers!
4. Use
setImmediate
for callbacks. Create errors withnew Error
.5.
result
is declared outside of this func. If you call this twice to download two files at the same time, the results will overwrite each other.7. Wrong way to do streams. Instead, catch the data event: http://nodejs.org/api/stream.html#stream_event_data
8. UTF8 chars can span multiple bytes. A given chunk might end in the middle of a UTF8 char, corrupting your data. Either buffer up all the responses and do the UTF8 decoding at the end or let node streams decode it for you with
stream.setEncoding('utf8')
http://nodejs.org/api/stream.html#stream_readable_setencoding_encoding11. Readable streams use
end
instead offinish
12. Should be
cb(null, result)
. Also, avoid useless comments.13. No handler for
onerror
event. If the connection is interrupted, your callback will never get called and this thread of execution will stall. (Thread isn't the right word here ... is there one?)14. Missing semicolon ;)