0
\$\begingroup\$

I have multiple places where I need to make ajax requests to fetch the items corresponding to some ids. However, I only want to make a single request by accumulating these ids and debouncing the actual method that makes the ajax request...So far I've come up with this code, but it just feels ugly/non-reusable.

Is there any simpler/recommended method to achieve similar results without sharing resolve/promise variables like I did here?

Here's a fiddle

const fakeData = [{
    id: 1,
    name: 'foo'
  },
  {
    id: 2,
    name: 'bar'
  },
  {
    id: 3,
    name: 'baz'
  }
];

let idsToFetch = [];

let getItemsPromise, resolve, reject;

const fetchItems = _.debounce(() => {
  console.log('fetching items...');
  const currentResolve = resolve;
  const currentReject = reject;

  // simulating ajax request
  setTimeout(function() {
    const result = idsToFetch.map((id) => fakeData.find(item => item.id == id));
    currentResolve(result);
  }, 400);

  getItemsPromise = resolve = reject = null;
}, 500);

function getItems(ids) {
  idsToFetch = ids.filter((id) => !idsToFetch.includes(id)).concat(idsToFetch);
  if (!getItemsPromise) {
    getItemsPromise = new Promise((_resolve, _reject) => {
      resolve = _resolve;
      reject = _reject;
    });
  }

  fetchItems();

  return getItemsPromise
    .then((res) => {
      return res.filter((item) => ids.includes(item.id));
    })
}

setTimeout(() => {
  console.log('first request start');
  getItems([1]).then(res => console.log('first result:', res));
}, 100);
setTimeout(() => {
  console.log('second request start');
  getItems([1, 2]).then(res => console.log('second result:', res));
}, 200)
setTimeout(() => {
  console.log('third request start');
  getItems([1, 3]).then(res => console.log('third result:', res));
}, 300)
<script src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.10/lodash.min.js"></script>

\$\endgroup\$
5
  • \$\begingroup\$ is this the actual code that you are using in your application? \$\endgroup\$
    – Malachi
    Commented May 8, 2018 at 14:03
  • \$\begingroup\$ @Malachi Other than the setTimeout that simulates the ajax request, then yeah it's pretty similar. \$\endgroup\$
    – Amr Noman
    Commented May 8, 2018 at 14:08
  • \$\begingroup\$ Code Review is about improving existing, working code. Unlike Stack Overflow, Code Review needs to look at concrete code in a real context. It is important that we have actual code to review. Please see Why is hypothetical example code off-topic for CR? \$\endgroup\$
    – Malachi
    Commented May 8, 2018 at 14:16
  • \$\begingroup\$ @Malachi I see, well...it's pretty much a working code from my project and the only different part is just substituting the ajax request because I don't have an online API at the moment to make the request to and it has nothing to do with what I'm asking about. Anyway I initially asked the same question on Stack Overflow and was told that it better fits here. \$\endgroup\$
    – Amr Noman
    Commented May 8, 2018 at 14:21
  • \$\begingroup\$ I assumed that the fakeData was test data similar to what would be passed in, I just wanted to make sure that the rest of the code was actual code. please feel free to add more context to what you are accomplishing with your code. \$\endgroup\$
    – Malachi
    Commented May 8, 2018 at 14:25

1 Answer 1

0
\$\begingroup\$

I was able to somehow encapsulate the logic be creating a function generator that holds the previous two functions like this:

  const fakeData = [{
      id: 1,
      name: 'foo'
    },
    {
      id: 2,
      name: 'bar'
    },
    {
      id: 3,
      name: 'baz'
    }
  ];

  function makeGetter(fetchFunc, debounceTime = 400) {
    let idsToFetch = [];

    let getItemsPromise, resolve, reject;

    const fetchItems = _.debounce(() => {
      console.log('fetching items...');
      const currentResolve = resolve;
      const currentReject = reject;
      const currentIdsToFetch = idsToFetch;

      Promise.resolve(fetchFunc(currentIdsToFetch))
        .then(res => currentResolve(res))
        .catch(err => currentReject(err));

      getItemsPromise = resolve = reject = null;
      idsToFetch = [];
    }, debounceTime);

    const getItems = (ids) => {
      idsToFetch = ids.filter((id) => !idsToFetch.includes(id)).concat(idsToFetch);
      if (!getItemsPromise) {
        getItemsPromise = new Promise((_resolve, _reject) => {
          resolve = _resolve;
          reject = _reject;
        });
      }

      fetchItems();

      return getItemsPromise
        .then((res) => {
          return res.filter((item) => ids.includes(item.id));
        })
    }

    return getItems;
  }

  const getItems = makeGetter((ids) => {
    // simulating ajax request
    return new Promise((resolve, reject) => {
      setTimeout(function() {
        const result = ids.map((id) => fakeData.find(item => item.id == id));
        resolve(result);
      }, 400);
    })
  });


  setTimeout(() => {
    console.log('first request start');
    getItems([1]).then(res => console.log('first result:', res));
  }, 100);
  setTimeout(() => {
    console.log('second request start');
    getItems([1, 2]).then(res => console.log('second result:', res));
  }, 200)
  setTimeout(() => {
    console.log('third request start');
    getItems([1, 3]).then(res => console.log('third result:', res));
  }, 300)
<script src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.10/lodash.min.js"></script>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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