3
\$\begingroup\$

I made the following code. Since it is a Batch, any kind of problems can appear. Please review it and point any problems when copying files into Dropbox! Its intent is that you will copy a file into an Dropbox folder. Its syntax is dropbox <files-to-dropbox> <dropbox-folder-to-copy-to>.

@echo off
set dropbox=%userprofile%\dropbox
if exist %dropbox% (if not exist %dropbox%\%2 mkdir %dropbox%\%2) else (echo Get Dropbox for PC before using this!)
if exist %dropbox%\%2 copy %1 %dropbox%\%2
@echo on

It would have been cool to use the dropbox tag as well, right?

\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

Your script will be a lot easier to read if you expand the long statements to multiple lines:

@echo off
set dropbox=%userprofile%\dropbox
if exist %dropbox% (
    if not exist %dropbox%\%2 mkdir %dropbox%\%2
) else (
    echo Get Dropbox for PC before using this!
)
if exist %dropbox%\%2 copy %1 %dropbox%\%2
@echo on

Is that @echo on at the end necessary? Probably not, but I don't have a Windows with me to play around to verify.

The last if statement is pointless to execute if the first if statement was false. It would be better to move this inside the first if statement:

@echo off
set dropbox=%userprofile%\dropbox
if exist %dropbox% (
    if not exist %dropbox%\%2 mkdir %dropbox%\%2
    if exist %dropbox%\%2 copy %1 %dropbox%\%2
) else (
    echo Get Dropbox for PC before using this!
)

Possibly an even better way is to use the equivalent of an early return, by leaving the main execution path when something is wrong. You can do this using goto :eof. After this the else statement can be flattened:

@echo off
set dropbox=%userprofile%\dropbox
if not exist %dropbox% (
    echo Get Dropbox for PC before using this!
    goto :eof
)
if not exist %dropbox%\%2 mkdir %dropbox%\%2
if exist %dropbox%\%2 copy %1 %dropbox%\%2

Rather than using the meaningless symbols %1 and %2 throughout the script, it would be better to put these values to variables with descriptive names, for example:

@echo off
set source=%1
set target=%2
set dropbox=%userprofile%\dropbox
if exist %dropbox% (
    echo Get Dropbox for PC before using this!
    goto :eof
)
if not exist %dropbox%\%target% mkdir %dropbox%\%target%
if exist %dropbox%\%target% copy %source% %dropbox%\%target%
\$\endgroup\$
1
  • \$\begingroup\$ Great! I loved your work with the script. Though later I might try Python, because I'm very good at it. \$\endgroup\$
    – wallabra
    Commented Nov 22, 2015 at 14:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.