Trying to follow Single Responsibility Principle with download and show progress

Question

Let's get right into it, I have a function downloadFile(url, path); Downloads file and saves it.

donwloadFile.js

module.exports = function donwloadFile(url, path) {
    return new Promise(res => {
        return axios({ url, method: 'GET', responseType: 'stream', })
            .then(({ data }) => {
                const file = createWriteStream(path);
                data.pipe(file);
                file.on('finish', () => {
                    file.close();
                    res({ ok: true });
                });
            })
            .catch(err => res({ ok: false, err }))
    });
}

Just there it already seems to violate SRP which i don't know what i can do about since the two functionality are part of each other, downloading and saving.

Ignoring that however, it's perfectly fine, it gets the job done and tells you what's going on.

to download a file you provide the url and the path where to save, simple.

---

However my main issue is that i want to add a progressBar (using progress library) to give some feedback and in order to do that i need to have access to data parameter of axios and the code becomes as follows.

donwloadFile.js

module.exports = function donwloadFile(url, path) {
    return new Promise(res => {
        return axios({ url, method: 'GET', responseType: 'stream', })
            .then(({ data }) => {
                const file = createWriteStream(path);
                data.pipe(file);

                const progressBar = new ProgressBar('[:bar] :percent | Time left: :etas.', {
                    complete: '=',
                    incomplete: ' ',
                    renderThrottle: 1,
                    total: parseInt(data.headers['content-length'])
                })
                data.on('data', (chunk) => {
                    progressBar.tick(chunk.length);
                })

                file.on('finish', () => {
                    file.close();
                    res({ ok: true });
                });
            })
            .catch(err => res({ ok: false, err }))
    });
}

and suddenly the download function isn't just downloading it's also spitting out a progressBar which is not ideal.

---

I had the idea of passing a callback which would be called inside data.on('data', callback)

But then we're doing the same thing again, the download function should only take two arguments, having a third one as callback makes it look like it's doing more than it says.

---

So my question is:

How can i keep the download function only downloads and also have the ability to show a progress bar ?

Also I'd welcome any healthy criticism, writing pretty and maintainable code has always been the bane of my existence, and i just got fed up and decided to learn some concepts.

Answer 1

Is SRP being violated?

I think the first version of donwloadFile.js is fine and obeys SRP. That said, I believe a cleaner solution is to refactor this method to return a file and then create a function that saves the file. I believe this is what @pacmaninbw suggested in his or her answer.

When to refactor?

If you find that you are copying the same lines to save a file in other parts of the code, you found yourself a refactoring opportunity. Now the question is how would you refactor. The way I have handled this in the past, and to stay true to Open/Close Principle (OCP), is to include a new version of the same method with this refactoring in place. In other words, I expand the functionality, instead of modifying it. That way, legacy code can continue to call the old downloadFile.js, while new code calls the new version. In fact, sometimes I would modify the legacy function where the legacy function will call the new function. To me, it is OK for legacy functions to depend on new functions (obviously, not the other way around).

What advantage does this approach have?

I can think of two:

1. You won't have to worry about failing test cases due to unforeseen side effects of this modification
2. If you ever retire the legacy code, you don't have to worry about dependencies (new functions don't depend on old functions).

So, in my humble opinion, even if downloadFile.js violates SRP, the best way to handle it might be leaving it alone and simply expand your functionality by adding a new version (or variant). That said, if the code is not in production yet, feel free to make all the modifications needed to have the cleanest implementation you could possibly get before your targeted release date.

Disclaimer: If you have core functions that you are OK with being part of the application forever, it is then OK for new functions to depend on these (core) old functions. SOLID Principles are recommendations, not gospel. It's like we striving for perfection. We will never get there no matter how hard we try. BUT, that should not stop up in trying our best to achieve it. In the end, the important thing is for us to be the best version of ourselves than we can be. Likewise, our code will NEVER be perfect. But, that should not stop us in developing the best applications we can. There are going to be situations when we would have to modify old code. We should be OK with that as long as it is not a common occurrence. The important thing is that, when opportunity present itself, we do a better job than the previous time to ensure these "violations" occur less often.