LOC per Dollar



  • My company outsources much of our coding to a cheap offshore company. By paying 1/6 the cost of a competent programmer, we've managed to get quite a bargain. Sure, they may wind up reimplementing functions like sprintf and snub their noses at using sql datatypes when varchar can be cast to whatever you need with ease, but that's no big deal since no one ever evaluates their work up close. Another benefit is many more LOC than we'd get with a full qualified programmer. Check out this little gem. This function is supposed to return a number as close to the target from the list.

    sub closest {
        my $target = shift;
        my %list = hash_with_keys(@_);
        my $closest;

        if (@_) {
            my @diffs = ();
            foreach (@_) {
                push @diffs, abs($_ - $target);
            }

            @diffs = sort { $a <=> $b } @diffs;

            my $gt = $target + $diffs[0];
            my $lt = $target - $diffs[0];

            if (defined($list{$gt})) {
                $closest = $gt;
            } else {
                $closest = $lt;
            }
        }

        return $closest;
    }


    sub hash_with_keys {
        my @list = @_;

        my %hash;
        foreach (@list) {
            $hash{$_} = 1;
        }

        return %hash;
    }

    I only have a little experience with perl myself, but even I can come up with something a little more succinct such as that below (though I claim almost no proficiency in the language):

    sub closest {
      my $target = shift;
      my @list = @_;
      @{[sort({abs($a - $target) <=> abs($b - $target)} @list)]}[0];
    }



  • @sp160n said:

    sub closest {
      my $target = shift;
      my @list = @_;
      @{[sort({abs($a - $target) <=> abs($b - $target)} @list)]}[0];
    }

    Write once, read never?
    Unreadable without comments. I can understand the original code, but got no idea what your code does... (I'll just believe it's the same) Are you sure, you want all your code to look like that?

    You can probably write every program in one line - but what's the point of that? 



  • @viraptor said:

    @sp160n said:

    sub closest {
      my $target = shift;
      my @list = @_;
      @{[sort({abs($a - $target) <=> abs($b - $target)} @list)]}[0];
    }

    Write once, read never?
    Unreadable without comments. I can understand the original code, but got no idea what your code does... (I'll just believe it's the same) Are you sure, you want all your code to look like that?

    You can probably write every program in one line - but what's the point of that? 

    I learned perl a long time ago and have not used it much, and it seems quite easy to understand... He simply sorts the array using a custom sort order based on the distance to the target. The result is then the first element.

    I'm not sure I would have done it that way. I think that sorting the array is useless. Why use a n.log(n) algorithm when you only need a sequential read throught the array?

    My way of writing it (untested pseudo code) :

    my candidateResult = array[0]
    my candidateDistance = abs(target - candidateResult)
    for element in array loop
        if (abs($target - $element) < $candidateDistance) then
            candidateDistance = abs($target - $element)
            candidateResult = $element
        end if
    end loop
    return candidateResult


  • @acne said:

    @viraptor said:
    @sp160n said:

    sub closest {
      my $target = shift;
      my @list = @_;
      @{[sort({abs($a - $target) <=> abs($b - $target)} @list)]}[0];
    }

    Write once, read never?
    Unreadable without comments. I can understand the original code, but got no idea what your code does... (I'll just believe it's the same) Are you sure, you want all your code to look like that?

    You can probably write every program in one line - but what's the point of that? 

    I learned perl a long time ago and have not used it much, and it seems quite easy to understand... He simply sorts the array using a custom sort order based on the distance to the target. The result is then the first element.

    I'm not sure I would have done it that way. I think that sorting the array is useless. Why use a n.log(n) algorithm when you only need a sequential read throught the array?

    My way of writing it (untested pseudo code) :

    my candidateResult = array[0]
    my candidateDistance = abs(target - candidateResult)
    for element in array loop
    if (abs($target - $element) < $candidateDistance) then
    candidateDistance = abs($target - $element)
    candidateResult = $element
    end if
    end loop
    return candidateResult

    Agreed.  You can search a list for a single item in O(n) time.  To do so in O(nlogn) is unacceptable and overly complicated.  Why sort everything when you just need one element?  And just for fun, here's a version based on bubble sorting (Assuming the array is indexed from 0 to array.length - 1):

    for element = 0 to array.length - 2
        if (abs(array[element] - target) < abs(array[element + 1] - target)) then
            swap = array[element]
            array[element] = array[element + 1]
            array[element + 1] = swap
        end if
    end for
    return array[array.length - 1]



  • @viraptor said:

    @sp160n said:

    sub closest {
      my $target = shift;
      my @list = @_;
      @{[sort({abs($a - $target) <=> abs($b - $target)} @list)]}[0];
    }

    Write once, read never?
    Unreadable without comments. I can understand the original code, but got no idea what your code does... (I'll just believe it's the same) Are you sure, you want all your code to look like that?

    You can probably write every program in one line - but what's the point of that? 


    Lemme help.

    sub closest {
      my $target = shift;
      my @list = @_;
      # got the arguments
    
      @list = sort { return abs($a - $target) <=> abs($b - $target) } @list;  
      return $list[0];   # returns list[0]
    }
    

    The original's @{[ ... ]}[0] construct-array-reference-then-dereference syntax is just a lil obscure, but if you've programmed Perl more than a week, it should be easy enough to understand. The implicit-return-last-value from the original deserves a little clarification in [code]sub closest[/code] but I think it's actually okay in the anonymous block passed to sort. Aside from the algorithmic complexity concerns... should we have written this in English because Perl is "unreadable" to some people? Or ought we be more concerned with your own Perl illiteracy?

    Are there any other questions?



  • Thanks for the tip. I can't believe I overlooked the totally obvious.


Log in to reply