Problem
Some days ago I’ve written JavaScript bot for reddit. All it does is replacing not well-formatted gfycat URLs. I would love to hear feedback about this code!
const chalk = require('chalk')
const Snoowrap = require('snoowrap')
const async = require('async')
console.log(chalk.cyan.bold('GfycatDetailsConvert is booting up...'))
let lastChecked
require('dotenv').config()
if (typeof process.env.REDDIT_USERAGENT === 'undefined') {
throw new Error('Please fill your .env file. For more information, go here: https://github.com/ImJustToNy/GfycatDetailsConvert')
}
const r = new Snoowrap({
userAgent: process.env.REDDIT_USERAGENT,
clientId: process.env.REDDIT_CLIENTID,
clientSecret: process.env.REDDIT_CLIENTSECRET,
username: process.env.REDDIT_USERNAME,
password: process.env.REDDIT_PASSWORD
})
r.config({
requestDelay: 2000,
continueAfterRatelimitError: true,
maxRetryAttempts: 5,
debug: process.env.NODE_ENV != 'production'
})
setInterval(() => {
r.getNew('all', {
before: lastChecked,
show: 'all',
amount: 1000
}).then(posts => {
if (posts.length > 0) {
lastChecked = posts[0].name
async.every(posts, (post, callback) => {
if (post.domain === 'gfycat.com' && /(/[a-z][a-z])?/gifs/detail/g.test(post.url)) {
post.fetch().comments.map(comment => comment.author.name).then(participants => {
callback(null, true)
if (!participants.includes(process.env.REDDIT_USERNAME)) {
console.log(chalk.red(chalk.bold('Found new post: ') + post.title + ' [/r/' + post.subreddit.display_name + ']'))
post.reply('[Proper Gfycat URL](' + post.url.replace(/(/[a-z][a-z])?/gifs/detail/g, '') + ') nn' + '^^I'm ^^just ^^a ^^bot, ^^bleep, ^^bloop. [^^[Why?]](https://gist.github.com/ImJustToNy/cb3457e36f22123eb93864f0af639da3) [^^[Source ^^code]](https://github.com/ImJustToNy/GfycatDetailsConvert)')
}
})
}
})
}
})
}, 5000)
Solution
Overall, it gets the job done, but your code (especially the area I’d refer to as “pyramid code”), could use a little more organization and clarity.
Your if
statement could be a little more concise, and, at the same time, more canonical about what it’s actually there for:
if (!('REDDIT_USERAGENT' in process.env)) {
...
}
You can write concise code without using cryptic variable names. Change r
to something more meaningful like robot
or even just bot
.
One way to resolve pyramid code is to look for places where your logic can be decoupled or flattened. While it’s possible to continue to use the async
dependency in order to achieve this, it’s probably much easier to just use async/await
since you’re already using promises anyway. This will also give you an opportunity to get rid of your global variable lastChecked
:
function matchesRequirements (post) {
return post.domain === 'gfycat.com' && /(/[a-z][a-z])?/gifs/detail/g.test(post.url)
}
async function tryReply (post) {
const { comments, subreddit, title, url } = await post.fetch()
const participants = comments.map(comment => comment.author.name)
if (!participants.includes(process.env.REDDIT_USERNAME)) {
console.log(chalk.red(`${chalk.bold('Found new post:')} ${title} [/r/${subreddit.display_name}]`))
const proper = url.replace(/(/[a-z][a-z])?/gifs/detail/g, '')
const reason = 'https://gist.github.com/ImJustToNy/cb3457e36f22123eb93864f0af639da3'
const source = 'https://github.com/ImJustToNy/GfycatDetailsConvert'
const text = `[Proper Gfycat URL](${proper}) nn^^I'm ^^just ^^a ^^bot, ^^bleep, ^^bloop. [^^[Why?]](${reason}) [^^[Source ^^code]](${source})`
return post.reply(text)
}
}
async function pollNewPosts () {
let lastChecked
while (true) {
const posts = await bot.getNew('all', {
before: lastChecked,
show: 'all',
amount: 1000
})
if (posts.length > 0) {
lastChecked = posts[0].name
}
await Promise.all(posts
.filter(matchesRequirements)
.map(tryReply)
)
await new Promise(resolve => setTimeout(resolve, 5000))
}
}
pollNewPosts()
One of the things in this rewrite to flatten your pyramid code, I changed the order in which .fetch()
is called, because the way your code accessed the comments
property relied on the usage of a proxy get trap in order to work properly, which is both unnecessary and inefficient.
Lastly, I cleaned up your generated text to break it up into a few lines, otherwise it’s quite unreadable. You might consider storing that text to an external template file using a minimal template engine (possibly written yourself just for this specific case) in order to avoid hard-coding it in here.