0

I am trying to refactor code where I use anti pattern but still don't understand how exactly to refactor something like this: I have a function that work like this now:

export async function submit(data) {
  return new Promise(async function(resolve, reject) {
    ...
    let list = await getList();
    ...
    for (let i = 0; i < list.length; i++) {
       let data = await getData(list[i]);
    }
    ...
    for (let i = 0; i < list.length; i++) {
       let response = await uploadFile(list[i]);
    }

And I call this function like:

let response = await submit(data);

I understand that async function in new Promise is problem but how to fix it?

export async function submit(data) {
  return new Promise(async function(resolve, reject) {
  let files = getFiles();

      let prepareUrlsResponse = await prepareUrls(paths);
      if (prepareUrlsResponse.error === true) {
        resolve( { success: false });
      } else {
        let blobs = [];
        for (let i = 0; i < prepareUrlsResponse.data.length; i++) {
          let fileData = await RNFetchBlob.fs.stat(filePath);
          blobs.push(fileData);
        }

        for (let i = 0; i < blobs.length; i++) {
          let item = blobs[i];

          let response = await uploadFile(
            item.url,
            item.blob
          );

          if (response && response.uploadSuccess === false) {
            resolve( { success: false });
            break;
          }
        }
      }


    api.post(
      endpoint,
      data,
      response => {
        if (response.ok) {
          resolve( { success: true, response: response });
        } else {
          resolve( { success: false });
        }
      }
    );
  });
}
1110
  • 7,829
  • 55
  • 176
  • 334
  • 1
    Please post the complete code of the function, most importantly the parts where you are calling `resolve` and `reject`. – Bergi Mar 17 '19 at 16:54
  • I updated the question. I removed some not relevant parts but keep all places where I resolve. I tried to remove `new Promise()` but when `return` I get `undefined`. – 1110 Mar 17 '19 at 17:13

1 Answers1

3

Just drop the line entirely. Your submit already is an async function, you can use await directly within it. There's no need to create a new Promise around the whole thing, you need it only to promisfy the api.post call. You should factor that out into a separate function returning a promise and use await on it like on the other functions you are calling.

function postData(url, data) {
  return new Promise(function(resolve, reject) {
//                   ^^^^^^^^ no `async` here!
    api.post(url, data, resolve);
  });
}

export async function submit(data) {
  let files = getFiles();
  let prepareUrlsResponse = await prepareUrls(paths);
  if (prepareUrlsResponse.error === true) {
    return { success: false };
  } else {
    let blobs = [];
    for (let i = 0; i < prepareUrlsResponse.data.length; i++) {
      let fileData = await RNFetchBlob.fs.stat(filePath);
      blobs.push(fileData);
    }

    for (let i = 0; i < blobs.length; i++) {
      let item = blobs[i];
      let response = await uploadFile(
        item.url,
        item.blob
      );

      if (response && response.uploadSuccess === false) {
        return { success: false };
      }
    }
  }

  const response = await postData(endpoint, data);
  if (response.ok) {
    return { success: true, response: response });
  } else {
    return { success: false };
  }
}

You could of course inline the postData call and use const response = await new Promise(…);, but you should only wrap the callback parts and not your whole logic.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • And add `try catch` blocks too :) – Code Maniac Mar 17 '19 at 16:58
  • Ah ok thanks `return instead of calling resolve(), throw instead of calling reject()` for this comment :) I thought that in order to `await` I need to have `new Promise` inside function. So when I call this should be `try{let res = await submit(data)} catch(err){}` so on error it will catch it here? – 1110 Mar 17 '19 at 16:59
  • @CodeManiac No? If you don't catch exceptions, the promise gets rejected, just as one would want. – Bergi Mar 17 '19 at 16:59
  • @1110 No, you need the `async` to use `await`, `new Promise` has nothing to do with that. And yes, you'd call it the same way as before. For more details, please edit your question so that I can extend my answer. – Bergi Mar 17 '19 at 17:00
  • I updated question am I doing everything wrong there? – 1110 Mar 17 '19 at 17:19
  • So that post is reason for me getting undefined then. And should I use throw error instead return { success: false}? In that case what ever fail in this function will end in catch where it was invojed? – 1110 Mar 17 '19 at 17:43
  • @1110 Yes, if throwing makes more sense to you than returning objects with `success` flags, it will work. – Bergi Mar 17 '19 at 18:07
  • Thank you it's much more clear what I need to understand better :) – 1110 Mar 17 '19 at 18:21