1
votes

I do not understand why my function to get the largest number does not want to work. If I am thinking about this correctly, if the first atom is smaller than the second atom then you call the function minus the first one in the list, else you construct the first atom, largest one, with the rest of the list. relevant code:

(define (getlargest a_list)
  (cond
    ((null? a_list) '())
    ((< (car a_list) (cadr a_list)) (getlargest (cdr a_list)))
    (else (cons (car a_list) (getlargest(cdr a_list))))))
7

7 Answers

4
votes

Your current procedure is failing at runtime. Even if it didn't, you're comparing one element with the next, but that won't work for finding a maximum, for instance in a list such as this it'll return 1, which is incorrect: '(10 2 0 1). There are other mistakes, for instance - why are you building a list as output, when the answer should be a number? also you have to be very careful with the edge cases, your procedure is failing when there's a single element left in the list.

The correct way is to compare one element assumed to be the maximum with all the rest, if we find one that is greater than the assumed maximum, then we've found a new maximum. This is what I mean:

(define (getlargest a_list)
  (if (null? a_list) ; edge case: empty list
      #f             ; return a special value signaling error   
      (let loop ((a_list (cdr a_list))   ; rest of the list
                 (maxval (car a_list)))  ; assumed maximum
        (cond ((null? a_list) maxval)    ; if the list is empty, return max
              ((> (car a_list) maxval)   ; current element > max
               (loop (cdr a_list) (car a_list))) ; found new max
              (else                      ; otherwise
               (loop (cdr a_list) maxval))))))   ; keep the same max

Of course, in real life we would use the built-in max procedure for the same purpose:

(apply max a_list)
3
votes

There are 2 errors in your code:

1) in the else clause, you should recursively call yourself, dropping the second element:

(else (getlargest (cons (car a_list) (cddr a_list))))))

2) you are missing the case of a list of only one element, where cadr will fail

((null? (cdr a_list)) (car a_list))

And I personally prefer to yield #f if the list is empty. Thus the code would look like

(define (getlargest a_list)
  (cond
    ((null? a_list)       
     #f)
    ((null? (cdr a_list))
     (car a_list))
    ((< (car a_list) (cadr a_list))
     (getlargest (cdr a_list)))
    (else 
     (getlargest (cons (car a_list) (cddr a_list))))))

Of course, a solution using foldl is preferable:

(define (getlargest lst)
  (foldl (lambda (e r) (if (or (not r) (> e r)) e r))
         #f
         lst))

or, probably slightly more efficient:

(define (getlargest lst)
  (if (null? lst)
      #f
      (foldl (lambda (e r) (if (> e r) e r))
             (car lst)
             (cdr lst))))
3
votes

This is incorrect because when you type (maximum '(2 1)) there is an error (any test with reverse order fails ) Without any loop, using recursivity:

(define (maximum L)
     (if (null? (cdr L)) 
         (car L) 
         (if (< (car L) (maximum (cdr L)))  
             (maximum (cdr L)) 
             (car L)
         )
    )
)
0
votes
(define (maxim lst)
  (vector-argmax (lambda (x) x) (list->vector lst)))
0
votes

First mistake is that you are returning list as output in base case. You need to return a number. You can implement this procedure in recursive way by storing the max value in MaxVal accumulator as we recurse through list.

Specification

input : List of positive unique numbers inList

output : Maximum value in list MaxVal

Follow is the implementation for above specification:

Procedure

(define (GetMaxVal MaxVal inList) 
    (cond ( (null? inList ) MaxVal )
          ( (> (car inList) MaxVal ) (GetMaxVal (car inList) (cdr inList)) ) 
          ( else ( GetMaxVal MaxVal (cdr inList)) ) ))

(define inList '(1 67 3 5 176 4745 34 575)) 

(display (GetMaxVal 0  inList) ) ; prints 4745 to screen

Explaination

  1. If list is empty, we have reached to base case hence return MaxVal
  2. If first element in list is greater than MaxVal than update MaxVal; pass MaxVal and (cdr inList) in argument in recursive call.
  3. else the first element is not greater so recurse by passing list without first element i.e. (cdr inList)
0
votes
;; ListOfNum Number -> Number
(define (list-maximum-core lon max)
  (cond
    [(empty? lon) max]   ; base case
    [else
      (list-maximum-core (rest lon) (if (> (first lon) max) (first lon) max) )] ))

;; ListOfNum -> Number
(define (list-maximum-main lon)
  (list-maximum-core lon 0) )
  • When calling the list-maximum-core function, make max = 0 as an initial value of max
  • On each recursive cycle of the list we check if the first item in the list is larger than max; if it is, this value is passed to the max variable; if not, the max variable value does not change
  • On each recursive cycle the first item in the list is removed and the list gets smaller; this continues until the list is empty, at which time it outputs whatever the value of max is at that time (when base case is true)
  • To avoid the inconvenience of adding 0 as a second variable every time you call the function, we create the list-maximum-main function
-1
votes
(define (max-list-element list)
  (define (aux list actual-max)
    (if (pair? (cdr list))
        (if (>= (car list) actual-max)
            (aux (cdr list) (car list))
            (aux (cdr list) actual-max))
        (if (> (car list) actual-max)
        (car list)
        actual-max)))
  (aux list (car list)))

My implementation.