-----------------------------------

TokenHerbz

Wed Dec 08, 2010 7:26 pm


FeedBack Please.

-----------------------------------

Being Very new to Ruby I'm unsure of the best ways to go about using certain loops and when i should compact code, etc etc..

I'm going to upload a picture which contains a challenge I've done, along with my code for you guys to give me feed back on.

Any tips or helpfulness would be wonderful, I want to learn the correct way and books/etc only take me so far, skilled programmers opinions are another valued source of learning.

First: the CHALLENGE: http://i.imgur.com/ieyDP.jpg

Second: The CODE  (please excuse the really long named variables, I got carried away!)


number1 = 0 #numbers the user will enter
number2 = 0
between_number_max_cycle_length = 0 #the value of the biggest cycle between number1 and number2

#will attempt to make it break down the number that is entered in, and inside it
#will count the number of cycles it goes threw, And this value will be returned
#to the number_cyle for the users number.
def break_down_number (number_,cycle_length_) #cycle_length is for users number (SET TO ZERO)
  if number_ % 2 == 0 #number is even
    number_ = number_ / 2 #divide it by 2
  else #odd number
    number_ = (number_ * 3) + 1 #times by 3 and add 1
  end
  cycle_length_ += 1 #add 1 to length of cycle each time we break down the number
  if number_ != 1 #if number not broken down to 1 yet then
    break_down_number(number_,cycle_length_) #algorithm continues
  else
    return (cycle_length_ + 1) #add 1 to cycle_length to include the last value
  end
end

#gets users numbers in range of 1 to 999,999
until number1 = 1
  puts "Please enter in 2 numbers from 1 to 999,999."
  number1 = gets.chomp.to_i
  number2 = gets.chomp.to_i #make the string INTS
  puts ""
end

#we go from number 1 up to number 2 and check each number for its cycle length
#if its larger then the MAX_length it is updated and kept...
number1.upto(number2) {|x|
  if break_down_number(x,0) > between_number_max_cycle_length
    between_number_max_cycle_length = break_down_number(x,0)
  end
}

#the output displayed exactly as challenge asks for...
puts "#{number1} #{number2} #{between_number_max_cycle_length}"



-----------------------------------

Insectoid

Wed Dec 08, 2010 9:04 pm


RE:FeedBack Please.

-----------------------------------

Your break_down_number method can be reduced a fair bit. 

def break_down_number(number_, cycle_length)
    number_ = number_%2 ? number_*3+1 : number/2
    cycle_length += 1
    if number != 1 
        break_down_number(number_, cycle_length_)
    end
    cycle_length_ += 1
end


I may have a syntax error or two, I didn't bother running this. 

Whenever you have something like 
[code]
if whatever then
   var = something
else
    var = something_else
end if
[/code]
It's (in my opinion) better to use the ? : structure, ie
[code]
var = whatever ? something : something_else
[/code]


-----------------------------------

jcollins1991

Wed Dec 08, 2010 9:15 pm


Re: FeedBack Please.

-----------------------------------

I'd suggest tying to write it with a while loop, I'm not sure how efficient Ruby is with tail recursion.


-----------------------------------

Tony

Wed Dec 08, 2010 9:26 pm


RE:FeedBack Please.

-----------------------------------

[code]
if number != 1
        break_down_number(number_, cycle_length_)
    end 
[/code]
can also be written as
[code]
break_down_number(number_, cycle_length_) unless number == 1
[/code]


-----------------------------------

wtd

Wed Dec 08, 2010 11:47 pm


RE:FeedBack Please.

-----------------------------------

Why not implement this as a method on the Fixnum class?

[code]class Fixnum
   def algorithm
      c = 1
      n = self
      while n != 1
         if n.even?
            n /= 2
         else
            n = n * 3 + 1
         end
         c += 1
      end
      c
   end
end[/code]


-----------------------------------

Tony

Thu Dec 09, 2010 12:22 am


RE:FeedBack Please.

-----------------------------------

Now that I have some more time to look over this:

- there is no need to initialize variables
- the typical convention for internal variables is _name instead of name_
- return doesn't need brackets for expressions
- the question states that the input would be valid, so the check isn't required, but if you must -- checking against ranges works too

@results = {1 => 1}

def max_cycle_for(n)
   while(@results


-----------------------------------

TokenHerbz

Thu Dec 09, 2010 6:45 am


Re: FeedBack Please.

-----------------------------------

Thank you everyone for the feed back.  

Insectoid : I like how to minimize those If statements and will be doing that more.  I wonder though, does it work with elsif too? or should I use a case? Or case is best suited for MULTIPLE ELSEIF's only?  What I've read on case is that it's really helpful or best used in really really large if structures but I'm sure it can be used for small ones to. But it doesn't say what the Ideal Situation would be to use them is... While learning ruby and coding I want to learn the correct way so I don't have poor programming habits later.

wtd : I've not even read on the ruby class's yet, but I will take a look in due time.  I feel I have way to much of the ruby basics to learn still. thanks tho.  I'm curious though, Do you add that to the actual FIXNUM class?... for future use in all programs?

Tony : Awesomeness.  I'll be going threw my ruby reads because I have no idea how some of that code works.  1 quick example would be [code] @results (1 => 1) [/code][/color]


-----------------------------------

Insectoid

Thu Dec 09, 2010 10:46 am


RE:FeedBack Please.

-----------------------------------

That conditional structure only supports binary conditionals. 

It's basically, 
 ?  : 

So, If/else structures only. 

I generally don't use case unless there's 3 or more unrelated values or if I need to run a different function based on input. An example of this is a basic calculater- there isn't any efficient way (I think) to assign *, /, + and - characters to their operations without a case or large, unwieldy if.

Also, I'm not quite sure but I think Ruby evaluates zero as false and anything else as true, so instead of  == 0 you can just do !.


-----------------------------------

jcollins1991

Thu Dec 09, 2010 11:38 am


Re: RE:FeedBack Please.

-----------------------------------

That conditional structure only supports binary conditionals. 

It's basically, 
 ?  : 

So, If/else structures only. 

I generally don't use case unless there's 3 or more unrelated values or if I need to run a different function based on input. An example of this is a basic calculater- there isn't any efficient way (I think) to assign *, /, + and - characters to their operations without a case or large, unwieldy if.

Also, I'm not quite sure but I think Ruby evaluates zero as false and anything else as true, so instead of  == 0 you can just do !.

The only false things in Ruby are 'nil' and 'false'

EDIT: Though, you can just do 0.zero? to get the falseness of 0 in other languages


-----------------------------------

wtd

Thu Dec 09, 2010 11:49 am


RE:FeedBack Please.

-----------------------------------

Yes.  Zero is a perfectly cromulent number, and in no way false.


-----------------------------------

Tony

Thu Dec 09, 2010 1:51 pm


RE:FeedBack Please.

-----------------------------------

results is a Hash -- http://ruby-doc.org/core/classes/Hash.html

it stores all the values that have already been calculated, to be used for future computations. This idea is the core of Dynamic Programming (bad naming, it's more of "programming while having a table of stored values"). Try to see what's stored inside after each call to max_cycle_for.

{1 => 1} is the base base. We know that the max cycle for n=1 is 1, per definition, so we initialize to this pair so that recursive loops terminate.


-----------------------------------

wtd

Thu Dec 09, 2010 1:52 pm


RE:FeedBack Please.

-----------------------------------

Dynamic programming is about doing as little work as possible by remembering where you've been before.


-----------------------------------

TokenHerbz

Sat Dec 11, 2010 2:15 pm


Re: FeedBack Please.

-----------------------------------

Another Programming Challenge Needing Feedback!  : I feel there's improvements, But I'll let you, the experts decide.

Here is the challenge: http://i.imgur.com/SZ8Dh.jpg
However, I went ahead and changed a few things myself:  I made it so you can adjust the grid size inside the code, And adjust the MINE PERCENTAGE for that grid.  The mines are also Randomly generated.

Please, be very critical and show me how how sloppy this is :)
I put if's inside the loops and the bottom if's i couldn't figure out how to compact that any way.  I gave it a lot of thought and this is the best i could come up with.


ROWS, COLUMNS = 5, 5 #change to edit the MINESWEEPER GRID SIZE
MINE_PERCENTAGE = 0.10 # 0.10 = 10%  // 0.50 = 50% (Make sure its ZERO . PERCENT
mine_tile = Array.new(ROWS) {Array.new(COLUMNS){"."}} #2D array with no mines
MINES = (MINE_PERCENTAGE * (ROWS * COLUMNS)).round #rounded amount of mines in grid
MINES.times{|mines| mine_tile


-----------------------------------

Tony

Sat Dec 11, 2010 4:44 pm


RE:FeedBack Please.

-----------------------------------

[code]
MINES.times{|mines| mine_tile[rand(ROWS)][rand(COLUMNS)] = "*"} #init random mines
[/code]
you don't need |mines| if you're not going to use it.
[code]
3.times {puts "ruby"}
[/code]
Also, strictly speaking, there is a chance of filling the same location with a mine twice. Meaning that at 100% MINE_PERCENTAGE, there could still be empty spaces.

[code]
ROWS.times{|rows| COLUMNS.times{|columns|
[/code]
sometimes it's easier to read do-end blocks
[code]
ROWS.times do |row|
   COLUMNS.times do |column|
      print ...
   end
end
[/code]

[code]
((rows - 1) = (ROWS - 1) ? (ROWS - 1) : (rows + 1))
[/code]
One trick to avoid such checks is to have a buffer around your array, so that you could safely go out of bounds. (Another would be to treat edges in a special way and use those as a buffer to safely work with everything else).


-----------------------------------

TokenHerbz

Sat Dec 11, 2010 5:21 pm


RE:FeedBack Please.

-----------------------------------

so have the array go from -1 to 5.

0,1,2,3,4 being the used array elements.
-1, 5 being NIL?

i'll try it out, i like that idea.


-----------------------------------

Tony

Sat Dec 11, 2010 7:02 pm


RE:FeedBack Please.

-----------------------------------

if you set the buffer to "." then everything should work well, and you wouldn't need a special case for nils.

Btw, index of -1 refers to the last element in the array.

>> 


-----------------------------------

Tony

Sat Dec 11, 2010 8:09 pm


RE:FeedBack Please.

-----------------------------------

and..

ROWS, COLUMNS = 5, 5
MINE_PERCENTAGE = 0.2
@mine_grid = Array.new(ROWS + 2){Array.new(COLUMNS+2){}}
# scatter some mines on the field
(1..ROWS).each do |r|
  (1..COLUMNS).each do |c|
    @mine_grid