Is there a good temporary variable name that you use?

#discuss #codequality #functional

Hi all,

I normally refer to variables as currValue, currentValue etc. I find it very intuitive myself. However, recently after seeing lot of people who don't find it clear, I would like to know if there's a better way to do so.

Following is the code where I use a temporary variable which I think can be named well.

function apiCall(type string, options Object(optional)) {
  // ... some code 
  // response format 
  // { nextPageToken <string>, items: <Array of Objects> }
}
function getAllFooBar() {
  // find smart_collections
  const response = apiCall('foo');
  const fooValueNextToken = response.nextPageToken;
  const fooValues = response.items;
  const barValues = [];
  // for every foo value
  for(let i=0; i< fooValues.length; i++) {
    const currFooValue= fooValues[i];
    let hasToken = true // setting true initially
    let token = null
    while(hasToken) {
      if (token!= null) {
        fooValue.token = token;
      }
      const innerResponse = apiCall('bar', currFooValue)
      // Is there a better way to name these variables?
      const currToken = innerResponse.nextPageToken;
      const currValues = innerResponse.items;
      barValues.push(...currValues); // saving the values received from api into allValues
      hasToken = currToken != null
      token = currToken
    }
  }
}

If you see the above code, I am getting some values from api and then iterating over to fetch another set of values.

I know this is not a question that's so important. But, I am someone who think that people modify code more often than they assume. So, I would like to know if there's a good way to make sure everyone is on the same page?

thanks in advance
cheers

Oldest comments (10)

Ben Calder • Oct 26 '20

My first suggestion would be to declare variable names that match the response object:

const items = response.items;

Then assuming your API has a sensible naming convention - i.e. a plural for groups of elements - you can use the singular in your iteration. But you should consider looping with an Array method instead; because then you can get rid of a lot of the temporary variables:

response.items.forEach(item => {
  // do stuff with each item
});

I should add that typically your API call will be async so you would expect to either use a Promise or an await there ;)
You would also usually add a guard to make sure you have a response to work with.

Venkatesh KL • Oct 26 '20 • Edited

Thanks Ben. I do understand that using array methods would make reduce the no. of temporary variables. I've kept it simple enough so that people coming from any language would understand. That's the reason I have removed await as well.
Also if it's in JavaScript, we may have to use something like bluebird to ensure all api calls are complete.

Also there are some instances where I'd like to have multiple tokens as in previous example. I will have a token for foo call, another for each of the api call for bar.
It can essentially become something like:
1 foos -> 1 bar requests(at least) -> 1 bar request can have multiple pages to query for.

Mihail Malo • Oct 26 '20

Do you use fooValueNextToken? or is there only ever one page of results for foo?

Venkatesh KL • Oct 26 '20

Ideally I would have multiple pages in such case the token would be used for fetching the next set of results. It's like a SQL cursor based pagination approach.

Mihail Malo • Oct 26 '20

Then, in the style of my other answer:

async function getAllFooBar() {
  const all = []
  for await (const foo of getPaged('foo'))
    for await (const x of getPaged('bar', foo))
      all.push(x)
  return all
}

Venkatesh KL • Oct 26 '20

Yep. That looks great. thanks.

Mihail Malo • Oct 26 '20 • Edited

I would avoid naming them, by doing the following:

function getAllFooBar() {
  // find smart_collections
  const foos = apiCall('foo').items
  const bars = []
  // for every foo value
  for (const foo of foos) {
    let token = undefined
    do {
      // undefined skips key in JSON stringification
      const barResponse = apiCall('bar', {...foo, token})
      bars.push(...barResponse.items)
      token = barResponse.nextPageToken
    } while (token)
  }
}

Usually I would abstract such a behavior that can probably be used for multiple paths of the API into a business-logic-indifferent function:

async function* getPaged(path: string, data?: any, token?: string) {
  do {
    const res = apiCall(path, {...data, token}).await
    for (const item of res.items) yield item
    token = res.nextPageToken
  } while (token)
}

async function getAllFooBar() {
  const all = []
  for (const foo of (await apiCall('foo')).items)
    for await (const x of getPaged('bar', foo))
      all.push(x)
  return all
}