0
\$\begingroup\$

I need to learn bash at some point in the near future, so I decided to start playing around with it. This is really the first actual script that I've written and would like feedback.

This finds and prints all the prime numbers up to the argument provided to the script:

./primes.sh 100
2
3
5
7
11
13
17
19
23
29
31
37
41
43
47
53
59
61
67
71
73
79
83
89
97

Honestly, this is proving to be a challenging language with very finicky syntax and a lot of ways to do things half-incorrectly. This code works, but I'm sure it's far from optimal. I'd like any and all recommendations.

I intentionally included the mostly-unnecessary hasFactor function just to force use of functions. I wanted to see what complications it caused. The main issue with it is needing to return a number representing a boolean that indicates if the number has a factor or not, and cleanly handling that returned exit code at the "call-site". Needing to check against 1 seems poor. Any suggestions regarding use of functions would be appreciated.

And I know better algorithms exist, but I wanted to keep it simple.

#!/usr/bin/env bash

maxNumber=$1

# Returns if a number has factors other than 1 and itself
# Params: n - number to check
function hasFactor {
    n=$1
    for ((factor=2; factor<n; factor++)); do
        if [ $((n % factor)) -eq 0 ]; then
            true
            return
        fi
    done

    false
    return
}



for ((n=2; n<maxNumber; n++)); do
    hasFactor $n
    if [ $? = 1 ]; then
        echo $n
    fi
done
\$\endgroup\$
3
\$\begingroup\$

Firstly, the algorithm is poor. Repeated trial division is one of the worst ways to generate primes. Prefer a sieve of some sort (you can use an array to store the primes).

Instead of true; return we can return 0; similarly, return 1 can replace false; return.

Testing $? is usually a sign that you simply need to move the tested command:

if ! hasFactor $n
then
    echo $n
fi

Or simply:

hasFactor $n || echo $n
\$\endgroup\$
2
\$\begingroup\$

You say: "This code works, but I'm sure it's far from optimal. I'd like any and all recommendations." AND "And I know better algorithms exist, but I wanted to keep it simple."

Unfortunately, these two are very hard to combine. If you want a piece of code to be efficient, a good algorithm is always the most important thing.

In general, my advice is to avoid doing logic in Bash. It is VERY clunky and it's extremely easy to make small mistakes that show up when you least expect it, and it can be years from now. I only use Bash for extremely simple logic. Basically, I treat it as a way of avoiding having to type commands manually in the command prompt. If you would not use the command prompt for a single instance of your problem, then Bash is probably the wrong language.

Your particular example is a case where I would use Python instead.

If you want to learn Bash, I STRONGLY recommend studying this link: https://mywiki.wooledge.org/BashPitfalls

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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