CODEWITHSUNDEEP
bash
HelloQuantumWorld
HelloQuantumWorld

Reputation: 157

What is wrong with this comparison that it won't see to compare to values correctly?

I am trying to find the oldest date (in string format) out of a list of elements. I can't figure out why the comparison is not working which results in the oldest date found always being the temporary oldest date as seen here:

filedates=(20200110 20200120 20200219 20200220 20200420 20200422 20200110 20200120 20200219 20200220 20200420 20200422 20200219 20200220 20200420 20200422)

find_oldest_date (){
    arr=("$@")
    currentOldestDate=${arr[0]} # set a temporary oldest date
    echo "Temporary oldest date: "$currentOldestDate
    for date in "${arr[@]}"; do
     currentDate=$date
    if [[ "$currentDate" -lt "$currentOldestDate" ]]; then
      oldestDateFound=$currentDate
     else
      oldestDateFound=$currentOldestDate
     fi
    done
    echo "Oldest date found in directory: "$oldestDateFound
}
find_oldest_date "${filedates[@]}"

for the comparison I've also tried:

 if [[ "$currentDate" < "$currentOldestDate" ]]

Or with only single brackets and double quotes, or double brackets no quotes ...

Its a requirement to pass filedates in as a parameter rather then use it directly

Upvotes: 0

Views: 37

Answers (4)

Paul Hodges
Paul Hodges

Reputation: 15293

Simplify.

$: filedates=(20200110 20200120 20200219 20200220 20200420 20200422 20200110 20200120 20200219 20200220 20200420 20200422 20200219 20200220 2020020 20200422)
$: find_oldest_date (){ printf "%s\n" "$@" | sort | head -1; }
$: find_oldest_date "${filedates[@]}"
20200110

Also...in your example, the first one is the oldest as far as I can see. Add a test value.

$: filedates=(20200110 20200120 20200219 20190220 20200420 20200422 20200110 20200120 20200219 20200220 20200420 20200422 20200219 20200220 2020020 20200422)
$: find_oldest_date "${filedates[@]}"
20190220

For efficiency -

find_oldest_date() {
  local oldest="$1"; 
  for d in "$@"; do [[ "$d" < "$oldest" ]] && oldest="$d"; done;
  echo "$oldest"; 
}

Upvotes: 0

KamilCuk
KamilCuk

Reputation: 141125

Note that you can just sort the arguments numerically:

find_oldest_date() {
   printf "%s\n" "$@" | sort -n | head -n1
}

Upvotes: 1

chepner
chepner

Reputation: 531235

You never change the value of currentOldestDate from its initial value of ${arr[0]}. As a result, you are reporting the last date seen that is less than ${arr[0]}, not necessary the oldest date seen.

find_oldest_date (){
    arr=("$@")
    oldestDateFound=${arr[0]}
    echo "Temporary oldest date: "$oldestDateFound
    for date in "${arr[@]}"; do
     currentDate=$date
    if [[ "$currentDate" -lt "$oldestDateFound" ]]; then
      oldestDateFound=$currentDate
     else
      oldestDateFound=$currentOldestDate
     fi
    done
    echo "Oldest date found in directory: "$oldestDateFound
}

You can simplify this to

find_oldest_date () {
  oldest=$1
  shift
  for current; do
    [[ $current -lt $oldest ]] && current=$oldest
  done
  echo "Oldest date found in directory: $oldest"
}

Upvotes: 1

Poshi
Poshi

Reputation: 5762

In your code, you are always comparing $currentDate (the actual date in the list you are traversing) against $currentOldestDate (which is just the first element of the array, it is not "current"). You must compare the $currentDate against an accumulator that contains the oldest date found so far and update as needed:

filedates=(20200110 20200120 20200219 20200220 20200420 20200422 20200110 20200120 20200219 20200220 20200420 20200422 20200219 20200220 20200420 20200422)

find_oldest_date (){
    arr=("$@")
    currentOldestDate="${arr[0]}" # set a temporary oldest date
    echo "Temporary oldest date: $currentOldestDate"
    for date in "${arr[@]}"; do
        if [[ "$date" -lt "$currentOldestDate" ]]; then
            currentOldestDate="$date"
        fi
    done
    echo "Oldest date found in directory: $currentOldestDate"
}
find_oldest_date "${filedates[@]}"

Upvotes: 0

Related Questions