1

So I'm writing a script to basically run my docker applications from quickly, I've got everything working just fine it does everything I coded it to do.

I just have a question about one of my functions:

function prompt_user() {
    echo "Enter details for docker build! If it's a new build, you can leave Host Directory and Remote Directory blank."
    echo "If you've already assigned variables and are running the host you can leave the already filled vars blank if you entered them before"
    echo " "
    echo "Enter details:"
    read -p "Image Name: " IMAGE_NAME
    read -p "IP Address: " IP_ADDRESS
    read -p "Port 1: " PORT_ONE
    read -p "Port 2: " PORT_TWO
    read -p "Container Name: " CONTAINER_NAME
    read -p "Node Name: " NODE_NAME
    read -p "Host Directory (Can leave this blank if you're building a new image): " HOST_DIRECTORY
    read -p "Remote Directory (Can leave this blank if you're building a new image): " REMOTE_DIRECTORY
}

Would there be an easier way to use read less repetitively and assign all the inputs to the vars?

Here is the full script if you'd like to look at it.

Improve this question
4
  • Yes, but would that make the script clearer? Commented Jul 5, 2018 at 11:31
  • Depends on the way you would be doing it. Commented Jul 5, 2018 at 11:32
  • Because this is going to be used for a range of different containers with different setups. I might build one and run a pre-built one before. This isn't finished, I'll be using getopts to have the execution run all on a single line. Commented Jul 5, 2018 at 13:11
  • Oh sorry, thought you meant in my main function. That's just a mistake :) When I copy from nano it usually pastes when right clicking and I must've accidently saved it. Been amended. Thanks. Commented Jul 5, 2018 at 13:16

2 Answers 2

Reset to default
3

I'm not sure how much cleaner this is than your existing function but using an associative array (requires bash v4.0 or later) combined with a for loop you could use read once. 

function prompt_user() {
    declare -A prompt_questions
    vars=(IMAGE_NAME IP_ADDRESS PORT_ONE PORT_TWO CONTAINER_NAME NODE_NAME HOST_DIRECTORY REMOTE_DIRECTORY)
    prompt_questions=(
        [IMAGE_NAME]='Image Name'
        [IP_ADDRESS]='IP Address'
        [PORT_ONE]='Port 1'
        [PORT_TWO]='Port 2'
        [CONTAINER_NAME]='Container Name'
        [NODE_NAME]='Node Name'
        [HOST_DIRECTORY]="Host Directory (Can leave this blank if you're building a new image)"
        [REMOTE_DIRECTORY]="Remote Directory (Can leave this blank if you're building a new image)"
    )
    cat <<EOF
Enter details for docker build! If it's a new build, you can leave Host Directory and Remote Directory blank.
If you've already assigned variables and are running the host you can leave the already filled vars blank if you entered them before

Enter details:
EOF
    for var in "${vars[@]}"; do
        read -rp "${prompt_questions[$var]}: " "$var"
    done
}
Improve this answer
5
  • Uppercase A on declare -A to make an associative array, otherwise the assignment takes the indices as numbers. Also you can use "${!prompt_questions[@]}" to get the keys of the array (IIRC), so you don't need the list in a separate array Commented Jul 5, 2018 at 17:07
  • @ilkkachu: Thanks I'm not sure why I swapped from uppercase to lowercase in a previous edit. I am using the second array with keys based on a suggestion from Kusalananda, because using the keys of an associative array will list them out of order which may be undesirable for OP. Commented Jul 5, 2018 at 17:09
  • @ilkkachu I take responsibility for the extra array! It's for ordering the questions. Commented Jul 5, 2018 at 17:09
  • @Kusalananda, oh right, of course! It's just the annoying need to list the names twice that irks me... Commented Jul 5, 2018 at 17:24
  • @ilkkachu Another way which would avoid listing the actual names twice would be to just use two ordinary arrays, one with variable names and one with question prompts. The position in the arrays would be what associates the variable name with the question. Commented Jul 5, 2018 at 17:27
1

I don't think your current code is that bad. The only repeating part is read -p, which is just a couple of characters. You can't get rid of the variable names or prompts, anyway.

(Though, there's the thing that someone might prefer command-line arguments instead of scripts that ask stuff interactively, but that's a question of preference.)

Anyway, since I said I didn't particularly like the double-listing of variable names @Jesse_b's associative array requires, here's another alternative:

prompt_user() {
    queries=(
        IMAGE_NAME='Image Name'
        IP_ADDRESS='IP Address'
        PORT_ONE='Port 1'
        PORT_TWO='Port 2'
        CONTAINER_NAME='Container Name'
        NODE_NAME='Node Name'
        HOST_DIRECTORY="Host Directory (Can leave this blank if you're building a new image)"
        REMOTE_DIRECTORY="Remote Directory (Can leave this blank if you're building a new image)"
    )
    echo "Enter details for docker build! If it's a new build, you can leave Host Directory and Remote Directory blank."
    echo "If you've already assigned variables and are running the host you can leave the already filled vars blank if you entered them before"
    echo " "
    echo "Enter details:"
    for query in "${queries[@]}"; do
        read -rp "${query#*=}: " "${query%%=*}"
    done
}

"${query#*=}" and "${query%%=*}" effectively split the string in query on the first equal sign.

Improve this answer

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.