task solution by IvanRyabukha · Pull Request #324 · mate-academy/node_move-files

IvanRyabukha · 2025-09-22T13:33:03Z

No description provided.

mateacademy-ai-mentor

Review Summary

Good start on this file moving utility! You've correctly handled the command-line arguments and basic path logic. However, I'm requesting changes because the current implementation deviates from several core technical requirements.

The most critical issue is that the move operation must be implemented by copying the file with streams (fs.createReadStream piped to fs.createWriteStream) and then deleting the source file (fs.promises.unlink). Your solution uses fs.renameSync, which does not meet this specific requirement.

Additionally, the task requires using asynchronous, promise-based methods like fs.promises.stat() to avoid blocking operations. Your code should be refactored into an async function that uses await instead of synchronous functions like fs.statSync.

Finally, please update the error handling to throw new Error(...) for all failure cases instead of logging to console.error, as specified in the requirements.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2025-09-22T13:35:23Z

src/app.js

+const fs = require('fs');
+const path = require('path');
+
+function move() {


Good start on the move function. However, the task requirements point towards an asynchronous implementation. You are expected to use promise-based methods like fs.promises.stat() and fs.promises.unlink().

Consider refactoring this function to be an async function. This will allow you to use await to handle asynchronous operations cleanly and is a best practice in Node.js to avoid blocking the event loop with synchronous I/O operations like fs.statSync or fs.renameSync.

mateacademy-ai-mentor · 2025-09-22T13:35:23Z

src/app.js

+
+  if (!fs.existsSync(sourceFile)) {
+    // eslint-disable-next-line no-console
+    console.error('Non-existent source file');


The task requires that an error must be thrown if the source file does not exist. Currently, you are logging a message to console.error and then returning from the function.

While this informs the user, it doesn't behave like a thrown error (e.g., it results in a successful process exit code 0). Please change this to throw new Error('Non-existent source file');. This applies to all the error conditions in your code.

mateacademy-ai-mentor · 2025-09-22T13:35:24Z

src/app.js

+
+  const dirName = path.dirname(destinationFile);
+
+  if (!fs.existsSync(dirName)) {


This check for the destination directory has a logical flaw. If destinationFile is a path like 'non-existent-dir/', path.dirname('non-existent-dir/') resolves to '.' (the current directory), which almost always exists. This check will pass, but the fs.renameSync call later will fail with an unhandled error because the directory doesn't exist.

You need a more robust way to validate the destination path. If destinationFile ends with a path separator, you should check if that path itself exists and is a directory.

mateacademy-ai-mentor · 2025-09-22T13:35:24Z

src/app.js

+  ) {
+    const newFilePath = path.join(normalizedDest, path.basename(sourceFile));
+
+    fs.renameSync(sourceFile, newFilePath);


This is the most critical issue. The task requires the "move" operation to be implemented as a two-step process:

Copy the source file to the destination using streams (fs.createReadStream piped to fs.createWriteStream).

Delete the original source file using fs.promises.unlink() only after the copy is successful.

Using fs.renameSync does not meet these specific technical requirements. You'll need to replace all instances of fs.renameSync with the stream-based copying logic followed by unlinking the source file.

mateacademy-ai-mentor

Review Summary

Thank you for your submission. While the argument parsing and path handling logic is well-implemented, I'm requesting changes because the core file operations do not yet meet the task's technical requirements.

Here are the main points to address:

Use fs.promises: The task requires using the asynchronous fs.promises API for all file system operations. Please convert your move function to be async and replace synchronous calls like fs.existsSync with their promise-based counterparts using await.
Implement Move via Copy-and-Delete: The use of fs.renameSync is not permitted. The move operation must be implemented by first copying the file using streams (fs.createReadStream and fs.createWriteStream) and then, upon successful copy, deleting the original file with await fs.unlink(sourceFile).
Throw Errors: Instead of logging errors with console.error, the application must throw an error in failure cases. For example, console.error('Non-existent source file'); should be replaced with throw new Error('Non-existent source file');.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2025-09-23T09:44:22Z

src/app.js

@@ -1 +1,77 @@
-// write code here
+const fs = require('fs');


The task requires using the asynchronous fs.promises API for all file system operations. Please import it instead of the synchronous fs module. You can achieve this by changing this line to const fs = require('fs').promises;.

mateacademy-ai-mentor · 2025-09-23T09:44:22Z

src/app.js

+const fs = require('fs');
+const path = require('path');
+
+function move() {


To correctly use the fs.promises API, this function should be declared as async. This will allow you to use await with the promise-based methods, making the code non-blocking and easier to read. For example: async function move() { ... }.

mateacademy-ai-mentor · 2025-09-23T09:44:22Z

src/app.js

+  if (!fs.existsSync(sourceFile)) {
+    // eslint-disable-next-line no-console
+    console.error('Non-existent source file');
+
+    return;


The requirements state that your application must throw an error in failure cases, not just log to the console. This is crucial for error handling in larger applications and for testing. Please replace the console.error and return with throw new Error('Non-existent source file');. This change should be applied to all error checks in this file.

mateacademy-ai-mentor · 2025-09-23T09:44:22Z

src/app.js

+  ) {
+    const newFilePath = path.join(normalizedDest, path.basename(sourceFile));
+
+    fs.renameSync(sourceFile, newFilePath);


As mentioned in the previous review, using fs.renameSync is not permitted for this task. The goal is to implement the move operation manually by copying the file and then deleting the original.

You should implement this by:

Creating a read stream for the source file (fs.createReadStream).

Creating a write stream for the destination file (fs.createWriteStream).

Using stream.pipeline to connect them, which handles the data transfer and error handling properly.

After the pipeline successfully completes, deleting the source file using await fs.unlink(sourceFile).

mateacademy-ai-mentor · 2025-09-23T09:44:22Z

src/app.js

+  const renameFile = path.join(
+    path.dirname(normalizedSource),
+    path.basename(destinationFile),
+  );
+
+  fs.renameSync(sourceFile, renameFile);


In addition to using the forbidden fs.renameSync method, there's a logical error in how the destination path is constructed here.

path.dirname(normalizedSource) resolves to the directory of the source file, not the destination. This means your code would attempt to rename the file within its original folder instead of moving it to the target location.

For example, mv ./dir1/fileA.txt ./dir2/fileB.txt would incorrectly result in ./dir1/fileB.txt instead of the expected ./dir2/fileB.txt. The correct destination for the new file is already available in your normalizedDest variable.

mateacademy-ai-mentor

Review Summary

Great job structuring the argument parsing and handling different path scenarios! I'm approving this submission as an educational step to help you move forward, but it's important to review the core requirements as they haven't been met yet.

Please focus on these key areas for your next task:

Asynchronous Operations: The task requires using the fs.promises module for all file system interactions. Your current code uses synchronous methods like fs.existsSync. Remember to make your functions async to use await.
Stream-Based Move: The use of fs.renameSync is explicitly forbidden. The main goal is to implement the move by first copying the file using fs.createReadStream().pipe(fs.createWriteStream()) and then deleting the original with await fs.unlink().
Correct Error Handling: Instead of logging errors with console.error, the application must throw new Error('message') to signal failure correctly.

Mastering these concepts is a crucial part of Node.js development. Keep up the great effort!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2025-09-23T10:05:00Z

src/app.js

@@ -1 +1,65 @@
-// write code here
+const fs = require('fs');


The task requires using the promise-based fs module. You should import it like this: const fs = require('fs').promises;. This will give you access to asynchronous methods that you can use with await.

mateacademy-ai-mentor · 2025-09-23T10:05:00Z

src/app.js

+const fs = require('fs');
+const path = require('path');
+
+function move() {


Since you'll be using await with methods from fs.promises, this function needs to be declared as async. For example: async function move() { ... }.

mateacademy-ai-mentor · 2025-09-23T10:05:00Z

src/app.js

+    console.error(
+      'Wrong number of arguments is supplied, should pass 2 arguments',
+    );


As mentioned in the previous review, the application must throw an error when something goes wrong, not just log to the console. This allows the calling code (or the Node.js runtime) to know that the operation failed. Please replace this with throw new Error('...');. This applies to all error handling in this file.

mateacademy-ai-mentor · 2025-09-23T10:05:00Z

src/app.js

+
+  const [sourceFile, destinationFile] = process.argv.slice(2);
+
+  if (!fs.existsSync(sourceFile)) {


This is a synchronous call. The requirements state that you must use the fs.promises API for file system operations. After importing fs.promises, you could check for file existence asynchronously. A common way is to try to access it and catch any error. For example: try { await fs.access(sourceFile); } catch { throw new Error('Non-existent source file'); }.

mateacademy-ai-mentor · 2025-09-23T10:05:00Z

src/app.js

+
+  const dirName = path.dirname(destinationFile);
+
+  if (!fs.existsSync(dirName)) {


There are two issues here. First, this is a synchronous call and should be replaced with an asynchronous one from fs.promises. Second, this logic is incorrect for destination paths that end with a separator, like nonexistent/. In that case, path.dirname returns . which always exists, and your check fails to catch that the nonexistent directory is missing. You'll need to handle this case specifically before attempting the move.

mateacademy-ai-mentor · 2025-09-23T10:05:00Z

src/app.js

+    fs.renameSync(sourceFile, newFilePath);
+
+    return;
+  }
+
+  if (destinationFile.endsWith(path.sep)) {
+    const newFilePath = path.join(normalizedDest, path.basename(sourceFile));
+
+    fs.renameSync(sourceFile, newFilePath);
+
+    return;
+  }
+
+  fs.renameSync(sourceFile, normalizedDest);


This is the most critical part of the feedback. The task explicitly forbids using fs.rename() or fs.renameSync. The 'move' operation must be implemented by:

Creating a readable stream from the sourceFile using fs.createReadStream().

Creating a writable stream to the new destination path using fs.createWriteStream().

Piping the read stream to the write stream to copy the file content.

Once the copy is complete (you can wrap the stream logic in a Promise), you must delete the original sourceFile using await fs.unlink(sourceFile).

Please replace all fs.renameSync calls with this stream-based copy-and-delete logic.

task solution

13234eb

mateacademy-ai-mentor suggested changes Sep 22, 2025

View reviewed changes

fix

05aab03

IvanRyabukha requested a review from mateacademy-ai-mentor September 23, 2025 09:42

mateacademy-ai-mentor suggested changes Sep 23, 2025

View reviewed changes

hotfix

be050e2

IvanRyabukha requested a review from mateacademy-ai-mentor September 23, 2025 10:03

mateacademy-ai-mentor approved these changes Sep 23, 2025

View reviewed changes


		const dirName = path.dirname(destinationFile);

		if (!fs.existsSync(dirName)) {


		const [sourceFile, destinationFile] = process.argv.slice(2);

		if (!fs.existsSync(sourceFile)) {

Conversation

IvanRyabukha commented Sep 22, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants