Skip to content

Develop #322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mForkosh wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Develop #322

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e64fd2c
solution
mForkosh Sep 6, 2025
ef65a7b
fix code
mForkosh Sep 6, 2025
6f0cf4a
...
mForkosh Sep 6, 2025
6ac6038
add fix
mForkosh Sep 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 48 additions & 1 deletion src/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,48 @@
// write code here
/* eslint-disable no-console */
'use strict';

const fsCb = require('fs');
const path = require('path');

const [subject, destination] = process.argv.slice(2);
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no validation for the number of arguments. The requirements specify that you must verify exactly two arguments are provided (source and destination), and throw an error if not. Please add this validation.


if (process.argv.slice(2).length !== 2) {
console.error('Usage: node index <source> <destination>');

process.exit(0);
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All error exits in this file use process.exit(0), which signals success. According to the requirements, error conditions (such as invalid arguments, missing files, or invalid destination) must exit with a non-zero code (e.g., process.exit(1)). Please change all process.exit(0) calls in error paths to process.exit(1) to properly signal errors as required.

}

const fileName = path.basename(subject);
let destinationIsDirectory = false;

try {
destinationIsDirectory = fsCb.statSync(destination).isDirectory();
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code does not handle the case where the destination ends with a slash (/) but does not exist. According to the requirements, if the destination is intended as a directory (ends with /) but does not exist, the app must throw an error and not create directories. Please add this check.

} catch (error) {}

const correctDestination = destinationIsDirectory
? path.join(destination, fileName)
: destination;

if (!destinationIsDirectory && destination.endsWith(path.sep)) {
console.error('Destination must be correct path');

process.exit(0);
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: this error exit should use process.exit(1) to indicate an error, not process.exit(0).

}

try {
fsCb.accessSync(subject, fsCb.constants.F_OK);

if (fsCb.statSync(subject).isDirectory()) {
console.error('Subject must be a file');
process.exit(0);
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: this error exit should use process.exit(1) to indicate an error, not process.exit(0).

}
} catch (subError) {
console.error(`subject doesn't exist`);
process.exit(0);
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: this error exit should use process.exit(1) to indicate an error, not process.exit(0).

}

try {
fsCb.renameSync(subject, correctDestination);
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no explicit check to ensure that only files are supported (not directories). The requirements specify that the app must not support moving directories. Please add a check to ensure the source is a file and not a directory, and throw an error otherwise.

} catch (err) {
console.error(err);
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor Sep 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Errors are only logged to the console, but the requirements specify that errors must be thrown and the process should indicate failure (e.g., exit with a non-zero code). Please update error handling to throw errors and exit with a non-zero code on failure.

}