Skip to content
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.

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

add solution #354

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

yaroslav1177
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Great job on implementing the file copy function! 🎉 While the code is mostly correct, there are a few areas for improvement, particularly in making error messages more descriptive. This will help users understand issues better, such as clarifying that the destination should be a file path, not a directory, and ensuring source and destination paths are different. Keep up the good work and consider these enhancements for future projects! 😊

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


const copyFile = (source, destination) => {
if (!source || !destination) {
throw new Error('Source and destination must be provided');

Choose a reason for hiding this comment

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

Consider providing more detailed error messages to help users understand what went wrong. For example, specify which parameter is missing.

}

if (source === destination) {
throw new Error('Source and destination paths are the same');

Choose a reason for hiding this comment

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

The error message could be more descriptive. Consider specifying that the source and destination paths should be different.

}

if (!fs.existsSync(source)) {
throw new Error(`${source} doesn't exist`);

Choose a reason for hiding this comment

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

Consider providing more information in the error message, such as suggesting the user check the file path.

const sourceStats = fs.statSync(source);

if (sourceStats.isDirectory()) {
throw new Error('Source is a directory, not a file');

Choose a reason for hiding this comment

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

The error message could specify that the operation is intended for files, not directories.

const destStats = fs.statSync(destination);

if (destStats.isDirectory()) {
throw new Error('Destination is a directory, not a file');

Choose a reason for hiding this comment

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

Consider clarifying in the error message that the destination should be a file path, not a directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants