Conditional async callbacks

Question

I'm writing an Electron program which takes a CSV file as input, and does file operations depending on the CSV content and file existence (it's to manage MAME arcade roms).

In order to have a progress bar on the UI side, I have switched the code from fully synchronous (because it was much easier) to asynchronous.

I just cannot find out how to reliably display a message to the user when all the lines in the CSV file are processed, and all the zip files are copied or removed.

Here is a (simplified) sample method:

fs.readFile(file, { 'encoding': 'utf8' }, (err, fileContents) => {
    let fileCsv = csvparse(fileContents);

    let lines = fileCsv.length;
    fileCsv.forEach((line) => {
        lines--;
        let zip = line.name + '.zip';
        let sourceRom = path.join(romset, zip);
        let destRom = path.join(selection, zip);

        this.emit('progress.add', fileCsv.length, fileCsv.length - lines, zip);

        if (fs.existsSync(sourceRom) && !fs.existsSync(destRom)) {
            fs.copy(sourceRom, destRom, (err) => {
                let sourceChd = path.join(romset, game);
                if (fs.existsSync(sourceChd)) {
                    fs.copy(sourceChd, path.join(selection, game), (err) => {
                        if (lines <= 0) { callback(); } // chd is copied
                    });
                } else {
                    if (lines <= 0) { callback(); } // no chd, rom is copied
                }
            });
        } else {
            if (lines <= 0) { callback(); } // no source found or already exists
        }
    });
});

The problem is that the CSV file is processed really fast, but the file are not copied as fast. So it decrements the lines counter to 0, then after each file copy, it finds that it's zero and triggers the callback.

How do I reliably trigger the callback at the end of all these nested callbacks and conditions?

Thanks

Just do the decrement after you actually finished to complete operation (with copying and everything)? — Bergi
– Bergi, Commented Apr 8, 2018 at 21:00
Don't ignore errors. You'll want to have a look at promises and async/await. — Bergi
– Bergi, Commented Apr 8, 2018 at 21:01
What do you mean after completing operations? Sometimes I do not do any operation and skip the entry. Sometimes there are 2 file operations, sometimes 1. It's a simplified example, I do not ignore the errors (for now I just rethrow them). I'm looking into promises and all but it's really a mess. — thomasb
– thomasb, Commented Apr 8, 2018 at 21:04
Yes, just put the lines-- at every place where you are finished with an entry, in every place where you have the if (lines <= 0) { callback(); }. — Bergi
– Bergi, Commented Apr 8, 2018 at 21:05
Also, it's an antipattern to do "file exists" checks. Filesystems are dynamic. The file might be there now and gone one tick later, or the other way around. The result of fs.existsSync() is not worth anything. Drop those checks. Copy the file. Handle the error. If you don't want to copy over an existing file, pass the fs.constants.COPYFILE_EXCL flag. — Tomalak
– Tomalak, Commented Apr 8, 2018 at 21:20

Catalyst · Accepted Answer · 2018-04-08 21:24:15Z

1

I tried to change the code without massively overwriting your style - assuming there is a reason to avoid things like bluebird, async/await & native Promises, and the async lib.

You need to decrement lines after a line is processed. I pulled the processing logic out into a function to make this clearer:

function processLine({
    sourceRom, destRom, romset, game, callback
}) {
    if (fs.existsSync(sourceRom) && !fs.existsSync(destRom)) {
        fs.copy(sourceRom, destRom, (err) => {
            // *really* should handle this error
            let sourceChd = path.join(romset, game);
            if (fs.existsSync(sourceChd)) {
                fs.copy(sourceChd, path.join(selection, game), (err) => {
                    // *really* should handle this error
                    callback();
                });
            } else {
               callback();
            }
        });
    } else {
        callback() // no source found or already exists
    }
}

fs.readFile(file, { 'encoding': 'utf8' }, (err, fileContents) => {
    let fileCsv = csvparse(fileContents);

    let lines = fileCsv.length;
    fileCsv.forEach((line) => {
        let zip = line.name + '.zip';
        let sourceRom = path.join(romset, zip);
        let destRom = path.join(selection, zip);

        this.emit('progress.add', fileCsv.length, fileCsv.length - lines, zip);

        processLine({ sourceRom, destRom, game, romset, callback: () => {
            lines--;
            if (lines <= 0) {
                callback();
            }
        }})
    });
});

answered Apr 8, 2018 at 21:24

Catalyst

3,23720 silver badges21 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

thomasb Over a year ago

Thanks! No reason for the "style", I'm not much of a JS coder, I come from C#. I switched to Promises (see my close/duplicate vote) but I still don't understand how they work. I discovered async/await tonight while searching for a solution. And I don't want to load more libraries than necessary.

Collectives™ on Stack Overflow

Conditional async callbacks

1 Answer 1

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related