-2

I have several functions in my ~/.bash_profile that I use as commands, and I've also created usage functions for them.

I'd like to know if I've implemented the usage functions correctly since I developed them on my own.

Example

IFS=$'\n'

usageSliceArr() {
  if [ $# != 3 ]; then
      echo 'name, index 1, index 2'
      return 1
  fi
 }

slicearr() {
  if (usageSliceArr $1 $2 $3); then 
      declare -n name=$1
      declare -i fst=$2 lst=$3
      echo ${name[@]:fst:lst}
  fi
 }
Nickotine
  • 364
  • 1
  • 11
  • Setting `IFS=$'\n'` doesn't remove the need to quote variables as as been said to you several times already. – Stéphane Chazelas Jun 29 '23 at 08:53
  • Errors should go to stderr. – Stéphane Chazelas Jun 29 '23 at 08:54
  • For number comparison, even though it will make no difference in this case, it's better to use `-ne` than `!=` which is a string comparison (`[ "$#" -ne 3 ]`). – Stéphane Chazelas Jun 29 '23 at 08:55
  • You'd do usage verification in functions that are intended for end-users, it's just unnecessary overhead if your function is only used in your script. – Stéphane Chazelas Jun 29 '23 at 08:57
  • `usageSliceArr $1 $2 $3` passes the first 3 provided arguments, subject to split+glob. You'd want to pass all the arguments with `usageSliceArr "$@"` for your usage function to detect when the user passes more than 3. – Stéphane Chazelas Jun 29 '23 at 08:58
  • Note that `(...)` is to spawn a subshell which in bash involves forking an extra process, so it's even more overhead. – Stéphane Chazelas Jun 29 '23 at 08:58
  • 5
    I’m voting to close this question because this seems to be a code review question without an actual problem to be solved, and should be posted on [codereview.se]. – muru Jun 29 '23 at 09:01
  • @StéphaneChazelas I'm not using any wildcards so don't have to worry about split+glob, as said in the description this is a part of my ~/.bash_profile which has many functions like this. If the reason to quote is because of security well it's a local script, I don't like the idea of do x all the time because it means your safe, when there are cases where it's fine not to do x. It's fine to encorce things but you know the reasons why, to others it creates an emotional response 'you didn't do x, you're bad'. It's a bit religious. – Nickotine Jun 29 '23 at 09:39
  • if I don't use (...) then I'd have to use $(...) which also spawns a subshell, so what do you propose? If I do $@ to detect if more than 3 args are passed then what about if less than 3 are passed? `$# != 3` covers both cases. – Nickotine Jun 29 '23 at 09:40
  • 2
    Should be `if usageSliceArr "$@"; then`, there's no need to use a subshell. – Stéphane Chazelas Jun 29 '23 at 09:41
  • 2
    `if [ $# = 3 ]` is a bit like saying `if (the splitting of number of argument results in a string that is 3)`. `if [ "$#" -eq 3 ]` is `if (the number of arguments is 3)`. It's about using the correct syntax of the language. – Stéphane Chazelas Jun 29 '23 at 09:42
  • then how about I use `((...))`? – Nickotine Jun 29 '23 at 09:45
  • Don't think this should be closed as I already got 2 sensible points that I wouldn't have thought about before and they might help others. – Nickotine Jun 29 '23 at 09:47
  • make that 3 sensible points. That there is a big difference between `-eq` and `=` – Nickotine Jun 29 '23 at 09:51
  • @Nickotine, the reason people keep reminding about quotes is that it's likely the single most frequent issue people ask about here, and also one that's not at all obvious. Other than changing history or forcing everyone to switch to some other language (even if just to zsh), trying to teach the habit of quoting by default seems best. Or well, least-worst. – ilkkachu Jun 29 '23 at 19:12
  • In any case, I wonder why you'd care about `-eq` vs `=` if not about quotes? You're only comparing for equality and you know `$#` is a properly formatted number so why not save keystrokes there, too and use `=`? More to the point, comparing `$#` with `=` won't even cause a surprise breakdown in the user's face regardless of what they provide as input, while passing something containing an asterisk or spaces just well might if the value is used unquoted. And that's kinda the point there, doing the right thing to avoid issues in the future. – ilkkachu Jun 29 '23 at 19:14
  • I understand the thing about quotes though it's best for everyone to quote I agree and at work if using bash scripts I know others will use I quote. You guys are the teachers you know all the reasons for quoting. Your students just follow, without knowing why. I can't be satisfied doing that, I want the knowledge the teacher has. You have to remember the reason I get away with not using quotes, and whitespace not effecting my code is because I always have `IFS=$'\n'` to stop word splitting. For `*` I'll be careful when I son't want globbing but usually for wildcards I only want globbing. – Nickotine Jun 29 '23 at 19:16
  • @ilkkachu Point noted about`=` and other symbolic operators, you're right it doesn't make a difference, just that when it was pointed out that when you go deep those operators do string evaluation it jolted me although it works. I remember I used to use the letter operators then didn't do bash for a while and forgot them but I never forgot the symbolic operators. – Nickotine Jun 29 '23 at 19:24
  • @Nickotine, oh, I don't really mean that people should just do what they're told without thinking or questioning. Quite the opposite, well, usually. It's just that here, the shell language gives you a deep enough pit that it might be easier for the student to climb out of it first, and then start figuring out the exact shape of the pit so they can skirt around if they like to. I.e. learn to do the safe the thing (quoting) first, so you can get things done, and then you can continue with the details. – ilkkachu Jun 29 '23 at 19:59
  • 3
    `IFS=$'\n'` doesn't stop word splitting, `IFS=` (setting the empty string) does. Yes, there's a difference, since the values your shell or shell functions get can contain newlines... Also neither does anything about stopping globbing, and stuff like `*?[]` are also something that can appear in inputs. Relying on "there's never going to be glob chars" fails when you build a script that e.g. passes patterns to `find -name`, or something like that. **Worst of all**, _all of those things can appear in filenames._ Now, they hopefully _won't_, but they _can_, sadly. – ilkkachu Jun 29 '23 at 20:04
  • 1
    I think there's a number of posts on the site that also tell you why something works and something else doesn't, and not just the what to do. [Why does my shell script choke on whitespace or other special characters?](https://unix.stackexchange.com/questions/131766/why-does-my-shell-script-choke-on-whitespace-or-other-special-characters), [When is double-quoting necessary?](https://unix.stackexchange.com/questions/68694/when-is-double-quoting-necessary), also see https://mywiki.wooledge.org/WordSplitting – ilkkachu Jun 29 '23 at 20:07
  • I have `find` functions I'll have to remember to set `noglob` for them, thanks for bringing that to my attention. I didn't say they stop globbing I just mean by being so loose I have learned about globbing and what to do about it. – Nickotine Jun 29 '23 at 20:11
  • exactly @ilkkachu I didn't say you were about dogma, I used to quote everything and anyone new should, when I had more understanding I started playing around. – Nickotine Jun 29 '23 at 20:15
  • @ilkkachu when I see the output of `IFS=$'\n'` with `bash -x` it comes up something like `IFS=$'` with the other quote on a newline (can't format that in the comments. Does this mean I have `IFS=''` and having `\n` is doing nothing and could actually catch me out if something has a newline? – Nickotine Jun 29 '23 at 20:41
  • 2
    @Nickotine, yes, it's a bit annoying it uses a hard newline in the `set -x` output. So, what you see is `IFS=''`. That's not the same as the empty string, or `IFS=''` – ilkkachu Jun 30 '23 at 06:19

2 Answers2

-1
  1. Double-quote your variables when you use them. Otherwise they'll break if you pass values that contain whitespace. Also, there's no need for that subshell. So instead of if (usageSliceArr $1 $2 $3); then you should write if usageSliceArr "$1" "$2" "$3"; then
  2. Use -ne to compare numbers, != for strings
  3. Don't forget there's https://shellcheck.net/
  4. Double-quote your variables when you use them
  5. Put the usage message inside the function that does the work, so that the code becomes self-documenting
  6. Write error messages to stderr rather than stdout
  7. Consider writing commands as shell scripts that live in your $PATH rather than functions available only if your ~/.bash_profile is executed
  8. Did I mention you should double-quote your variables when you use them?

Here's one approach:

########################################################################
# slicearr name index count
#
# Return <count> space-separated and tokenised elements from the <name>
# array starting at <index>
#
slicearr() {
    if [ $# -ne 3 ]
    then
        echo 'Usage: name index count' >&2
        exit 1
    fi

    declare -n name=$1
    printf "%s " "${name[@]:$2:$3}"; echo
}

The relevance of the space-separated and tokenised clause applies to your original code, too. Consider an array

a=( one two 'twenty three' )

The "twenty three" will be extracted as a single element but returned as two space-separated values "twenty" and "three".

roaima
  • 107,089
  • 14
  • 139
  • 261
  • I'm pretty sure in point 1 you can't write the if statement that way, and I do have white space, nothing breaks as `IFS=$'\n' ` I've got it set in ~/.bash_profile and I never had any issues with white space in scripts or in the terminal. I like the use of printf if it means I don't need to do `declare -i` but I'll have to test that. For the array `a` it's a special case that I'd never do but I'm pretty sure that twenty and three wouldn't split, the way I've set IFS prevents word splitting. Not a fan of the comment, I think the code should speak for itself. I like the redirection to error. – Nickotine Jun 29 '23 at 13:04
  • think I'm wrong about the way you can write the if statement, I'll test that, I have a vague memory that I tried that I'll have to test. The reason I don't like the comment is because `slicearr()` tells you what the function does. Also I separate the usage from the `slicearr` to keep to the idea a function does 1 thing. – Nickotine Jun 29 '23 at 13:12
  • 1
    @Nickotine `if (usage $1 $2 $3)` puts the word `usage` and the globbed expansion of the three arguments `$1`, `$2`, and `$3` into a subshell (i.e. `( ... )`) before evaluating it. There's no need to invoke a subshell so just test it directly – roaima Jun 29 '23 at 18:52
  • yes you're right thank you. – Nickotine Jun 29 '23 at 18:53
-2

Using some good suggestions from @StéphaneChezalas, I'm going to do this now:

IFS=$'\n'

usageSliceArr() {
  if (( $# != 3 )); then
      echo 'name, index 1, index 2'
      return 1
  fi
 }

slicearr() {
  usageSliceArr $@ &&
    declare -n name=$1
    declare -i fst=$2 lst=$3
    echo ${name[@]:fst:lst}
 }

Bear in mind I have many functions including this one in my ~/.bash_profile it's not just a one off script.

Nickotine
  • 364
  • 1
  • 11
  • `!=` should be `-ne` as you're comparing numbers rather than strings. `usageSliceArr $@` should be `usageSliceArr "$@"` to avoid globbing and splitting of the arguments. Likewise `echo ${name[@]:fst:lst}` should (at least) be `echo "${name[@]:fst:lst}"`. But consider what would happen if the first extracted value happens to be `-r`... – roaima Jun 29 '23 at 18:56
  • don't worry I've got IFS set to stop word splitting on whitespace and I'm not worried about globbing since I don't see a scenario where I'll pass a wild card that I want to be taken literally. I'm using ((..)) which is for arithmetic, i just don't like `-ne` and find symbols much easier to remember. The globbing problem is really an edge case for this function. I can't see a situation where I'd have a wildcard chsracter expanded since the data wouldn't include such things. If I was doing official work I'd take your suggedtion. – Nickotine Jun 29 '23 at 18:58
  • "_I'm not worried about globbing since I don't see a scenario where [...it will happen...]_" - experienced programmers (aim to) write defensive code. Read up on Murphy's Law some time. – roaima Jun 29 '23 at 19:01
  • "_i just don't like `-ne`_" - if you want to continue to write incorrect code, good luck to you – roaima Jun 29 '23 at 19:02
  • `!=` is valid in `((..))` since its for arithmetic. This is just my personal stuff so I'm being loose. – Nickotine Jun 29 '23 at 19:03
  • `!=` compares values as strings – roaima Jun 29 '23 at 19:07
  • in fact by being so loose with my personal stuff, I have learned a lot; globbing, a great understanding of the security risks of not using quotes (not just; no quotes=bad) I mean the exact scenarios, why and when to use `set noglob` I didn't mean to learn all this in so much detail, but I prefer to know exactly what the teacher knows rather than following a dogma. – Nickotine Jun 29 '23 at 19:09
  • 1
    @roaima, `(( ))` is an arithmetic context, there's only `==`/`!=`/`<` etc, which compare numbers. `(( 8+1 == 10#09 ))` is truthy. – ilkkachu Jun 30 '23 at 06:23
  • schooled @roaima 'the dogmatist, entrenched in the old ways.' by ilkkachu 'the wise and free.' – Nickotine Jun 30 '23 at 22:31