using cp in shell script function () does not work

Question

mkdir works fine but it does not copy (doesn't throw an error either).

#!/bin/sh

copy_function() {

if $1 $2 $3 $4;then
    echo "copied directory and contents"
else
    echo "failed"
    exit 1
fi
}

copy_function mkdir -p ~/TestFolder/TestFile/
copy_function cp -pr my_data/* /TestFolder/TestFile/

Answer 1

copy_function() {
if $1 $2 $3 $4;then
    echo "copied directory and contents"
else
    echo "failed"
    exit 1
fi
}
copy_function mkdir -p ~/TestFolder/TestFile/
copy_function cp -pr my_data/* /TestFolder/TestFile/

Let's deconstruct this function. What does it actually do?

Firstly, it expects four arguments; if given more than that it will ignore all but the first four.

The line beginning with if means:

Subject the first four positional parameters (arguments received by this function) to file glob expansion, followed by word splitting on white space, and then attempt to run the resulting line as a command (with the first word used as a command and the remainder passed as arguments to that command).

A better way to do this, using all positional parameters, and not subjecting them to an extra file glob expansion pass and word splitting, would be:

if "$@"; then

The next line outputs a message, "copied directory and contents". The problem with this line is that it's misleading in the extreme—that's not what the function did. The function did whatever was expressed in the first argument received. If I were to run copy_function rm -r -f ~, I would actually be deleting my home directory—but I would get the output message, "copied directory and contents." So this line is a lie.

The next two lines, else and echo "failed" are at least more accurate, since they don't try to say what failed. Ideally error output would be delivered to standard error instead of standard out, though:

else
    echo failed >&2

The exit 1 command might be okay for use in a script, if you really want your whole script to exit right at that point. It might be better to use return 1 instead, but this is a quibble.

The bigger problem is that this entire function is totally pointless.

There is absolutely no use case for it ever.

What does it do? It:

1. Uses some (but not necessarily all) of its arguments,
2. Does file glob expansion and word splitting on those (which you probably didn't know about),
3. Runs them as a command, and
4. Prints a message which is probably a lie if the command succeeds, and
5. Prints a totally uninformative message "failed" if the command fails.

NONE of this is useful functionality.

The script could be shortened to just this:

mkdir -p ~/TestFolder
cp -vpR mydata ~/TestFolder/TestFile

This still has its own oddities, one of which is your transition from a home-directory-based path (~/TestFolder) to a root-directory-based path (/TestFolder), but I've fixed that. An odder choice is the name TestFile for a directory.

But those aspects aside, note the -v switch to make the copy command verbose.

Also note that by copying the entire containing directory, rather than a glob expansion, hidden files (a.k.a. "dotfiles") will be copied regardless of the current setting of your shell's "dotglob" option.

---

A properly written shell script is very concise, and very powerful. Don't reinvent the wheel. Learn the features of the commands you're using before adding wrapper functions.

And please, read the Wooledge Bash Guide if you plan to do any serious scripting.