0
\$\begingroup\$

This is a pretty simple function that returns a value based on the variable category. I tried to use switch instead of define menu object that contained functions with tag name but I noticed that it looks pretty dirty so that's how I made this code. After that I realized defining menu variable will run every function in the object to store the value in the properties which is not what I'm trying to do. What I want to do is that I want to run only a function and it needs to run to get a value based on the variable category. How could I make this better?

const getContent = async (category, tag) => {
    let query = { 'tag':tag, 'limit':10 }

    let menu = {
        'feed': () => steem.api.getDiscussionsByFeed(query),
        'blog': () => steem.api.getDiscussionsByBlog(query),
        'new': () => steem.api.getDiscussionsByCreated(query),
        'hot': () => steem.api.getDiscussionsByHot(query),
        'trend': () => steem.api.getDiscussionsByTrending(query)
    }

    return await menu[category]()
}
\$\endgroup\$
  • \$\begingroup\$ Defining the menu object does not run any of the functions that are assigned as properties. It just defines those functions. \$\endgroup\$ – jfriend00 Nov 27 '17 at 4:30
1
\$\begingroup\$

After that I realized defining menu variable will run every function in the object to store the value in the properties which is not what I'm trying to do.

What you have looks like it would work just fine. Declaring the menu structure does not execute the functions you've assigned as properties. It just defines the functions and assigns them as properties (waiting to be called later).

You could save a little execution overhead by moving the static menu definition outside the function so it just gets defined once (I'm assuming this is private module scope so moving the symbol outside the function is fine) and change it to const.

You can also remove the async and await since neither add anything here. You're just returning a promise anyway so you can just return the promise directly without async and await. I also added more explicit error checking for the category name.

const methodMap = {
    'feed': () => steem.api.getDiscussionsByFeed(query),
    'blog': () => steem.api.getDiscussionsByBlog(query),
    'new': () => steem.api.getDiscussionsByCreated(query),
    'hot': () => steem.api.getDiscussionsByHot(query),
    'trend': () => steem.api.getDiscussionsByTrending(query)
};

const getContent = (category, tag) => {
    const query = { 'tag':tag, 'limit':10 };
    if (methodMap.hasOwnProperty(category)) {
        return methodMap[category]();
    } else {
        return Promise.reject(new Error(`Unknown Category: ${category}`));
    }
}

You could also remove some redundancy from the table by making the table only be the steem method name you want to call.

const methodMap = {
    'feed': 'getDiscussionsByFeed',
    'blog': 'getDiscussionsByBlog',
    'new': 'getDiscussionsByCreated',
    'hot': 'getDiscussionsByHot',
    'trend': 'getDiscussionsByTrending'
};

const getContent = (category, tag) => {
    const query = { 'tag':tag, 'limit':10 };
    if (methodMap.hasOwnProperty(category)) {
       return steem.api[methodMap[category]](query);
    } else {
        return Promise.reject(new Error(`Unknown Category: ${category}`));
    }
}

This layout also does more explicit error checking on a "supported" category being passed in.

\$\endgroup\$
  • \$\begingroup\$ Shouldn't 'feed': () => steem.api.getDiscussionsByFeed(query), be 'feed': steem.api.getDiscussionsByFeed, in your first example? \$\endgroup\$ – le_m Nov 27 '17 at 4:49
  • \$\begingroup\$ @le_m - No. That would lose the steem.api reference when calling the method (in case that matters). But inside of the first getContent() it should have just been return methodMap[category](); which I've fixed it to. \$\endgroup\$ – jfriend00 Nov 27 '17 at 4:51
  • \$\begingroup\$ Ah right, one would have to write methodMap[category].call(steem.api, query) which isn't too readable. \$\endgroup\$ – le_m Nov 27 '17 at 4:56
  • 1
    \$\begingroup\$ @le_m - And, for the first option, I was trying to preserve the table as they had it, but just move it out of the function for efficiency. My second option reduces the table to less redundancy. \$\endgroup\$ – jfriend00 Nov 27 '17 at 4:57
0
\$\begingroup\$

I don't know why you think switch looks dirty, I think it's perfect for this situation. You can drop the break, since you are returning anyway, and each case is just one function call, so you can put it all on one line. It's also easy to handle errors by just adding a single line.

const getContent = async (category, tag) => {
    let query = { 'tag':tag, 'limit':10 }

    switch(category) {
        case 'feed':  return steem.api.getDiscussionsByFeed(query);
        case 'blog':  return steem.api.getDiscussionsByBlog(query);
        case 'new':   return steem.api.getDiscussionsByCreated(query);
        case 'hot':   return steem.api.getDiscussionsByHot(query);
        case 'trend': return steem.api.getDiscussionsByTrending(query);
        default: return 'Error';
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.