From: Robert Klemme <shortcutter@...>
Date: 2010-12-02T23:25:45+09:00
Subject: Re: Help sorting an array

On 01.12.2010 19:30, Mike Stephens wrote:
> If you've ever read "Real Programmers don't use Pascal" (see
> http://www.codeproject.com/KB/scrapbook/realprog.aspx ) you'll know that
> "Determined Real Programemrs can write Fortran programs in any language"
> Just because you're a Ruby programmer you don't have to use all that OO
> stuff.

That's true.  However, I would add that the choice of language does not 
determine the algorithm to use nor how it is implemented.  We'll come to 
that in a minute.

> This algorithm picks out the first label and moves it into a new space
> on the left, and then moves the second label to where the first was and
> so on. Lastly it removes the vacated slot on the right:

Here we see the first indication of a bad implementation: the number of 
labels is encoded in the algorithm.  You cannot use that program with 
other than three labels.  Even worse, the exact labels are hard coded 
into the program.  In other words, your program is overly dependent on 
the input, or - put differently - your program is not very general and 
poses too many restrictions on the input.

> def initialise_globals
>    $m1 = "JANUARY"
>    $m2 = "FEBRUARY"
>    $m3 = "MARCH"
>
>    $m1_index = 0
>    $m2_index = 0
>    $m3_index = 0
> end

Others have commented on the use of global variables already.

> def get_array
>    $input_array = ["#<Event:0x4d4cf78>", "#<Event:0x4d4d470>", "MARCH",
> "#<Event:0x4d4dc08>",
> "#<Event:0x4d4e100>", "FEBRUARY", "#<Event:0x4d4e898>","JANUARY"]
>    puts " #{$input_array}"
> end
>
> def shuffle_all_right
>    #insert m3 while we’re at it.
>    $input_array.unshift " "
> end

Isn't this "OO stuff" that you did not want to use?

> def locate_months
>    $m1_index = $input_array.index $m1
>    $m2_index = $input_array.index $m2
>    $m3_index = $input_array.index $m3
> end

Here you are clearly cheating: you use "all that OO stuff".  Even worse, 
by using Array#index you are hiding the fact that you traverse the input 
Array far more often than necessary.

> def bring_months_to_left
>    $input_array[0] = $input_array[$m3_index]
>    $input_array[$m3_index] = $input_array[$m2_index]
>    $input_array[$m2_index] = $input_array[$m1_index]
> end
>
> def delete_right_slot
>    $input_array.pop
> end

Again, "OO stuff".

> def main_program
>    initialise_globals
>    get_array
>    shuffle_all_right
>    locate_months
>    bring_months_to_left
>    delete_right_slot
> end
>
> main_program
>
> puts $input_array
>
> Actually I cheated there as I enclosed the #strings in quotes. Anyone
> know how to deal with #s?

If you want to make the point that using Ruby does not necessitate the 
usage of "all this OO stuff" a better program (i.e. one that at least 
avoids issues raised so far) would certainly have given your argument 
more strength.

> I believe the program I wrote whilst not typical of Ruby programs is
> quicker to understand than the more dense solutions given by others -
> and that's important in programming.

Well, then compare that to the version below.  I claim that it uses 
(mostly) procedural style and yet is easier to read and understand than 
your version.


# test whether the given element is a month label
def is_label(o)
   String === o
end

# exchange values at positions
def swap(arr, i, j)
   # not using parrallel assignment since
   # we want to avoid all the fancy stuff
   x = arr[i]
   arr[i] = arr[j]
   arr[j] = x
end

# Input: array with Events followed by month
# Output: number of events without month at end
# Side effect: months precede their Events
def realign_months(arr)
   start = 0
   i = 0

   while i < arr.length
     if is_label(arr[i])
       # swap label and first
       swap(arr, i, start)

       # remember begin of next section
       start = i + 1
     end

     i += 1
   end

   # remaining elements without month
   return arr.length - start
end

# we add a field only for easier reading of output
Event = Struct.new :no

arr = [
   Event.new(1),
   Event.new(2),
   Event.new(3),
   "January",
   Event.new(4),
   "February",
   Event.new(5),
   Event.new(6),
   Event.new(7),
   "March",
   Event.new(8),
   Event.new(9),
   "April",
]

p arr

remainder = realign_months(arr)

p arr

if remainder > 0
   $stderr.puts "Last #{remainder} events are misaligned"
end

I would readily admit that it does not exactly do the same thing as your 
program.  You can find this version and a version that maintains 
original element ordering (such as your version does) here: 
https://gist.github.com/725368

Kind regards

	robert

-- 
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/