current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

This was a challenge for reddit #202 Easy /u/learnprogramming ...I wish that someone reviews my code and let me know your opinion about my way of thinking beacause I am a total beginner in Ruby:

class BinaryChar
  attr_accessor :binaryString

  def convertToChar
    binaryString.scan(/.{1,8}/) { |part| print part.to_i(2).chr }
    puts
  end

end

bc = BinaryChar.new
speak = 0 
loop do
  print "Enter string: "
  speak = gets
  bc.binaryString=(speak)
  bc.convertToChar
end
share|improve this question

1 Answer 1

up vote 1 down vote accepted

BinaryChar is a class with one method. You continually change the state of the object so you can call the method with the right "argument". That's not what classes are for, but module functions or class methods. I rewrote the code twice: once using a class in the right way and once using a module function (or class method).

with a class

class BinaryChar
  def initialize(string)
    @binary = string.scan(/.{1,8}/).map do |character|
      character.to_i(2).chr
    end
  end

  def to_s
    @binary.join
  end
end

loop do
  print "Enter string: "
  bc = BinaryChar.new(gets)

  puts bc # to_s is called automatically by puts.
end

with a module function

module Binary
  def self.to_binary(string)
    binary = string.scan(/.{1,8}/).map do |character|
      character.to_i(2).chr
    end

    binary.join
  end
end

loop do
  print "Enter string: "

  puts Binary.to_binary
end

A couple of minor nitpicks:

  • In ruby the convention is to use snake_case for anything but ClassNames and CONSTANTS.
  • When you get input from stdin (gets) a newline is appended to it ("\n"). In most cases you want to strip the newline of using String#chomp.
  • Accessor methods can look just like variable assignments: bc.binary_string = speak.
  • Conversion methods are usually named to_<something>. To convert to a String, for example, most classes implement a to_s method.
share|improve this answer
    
In your first example, you create a new object every time you get a new binary code from the keyboard. Isn't it a waste of resources? I thought exactly this - I created the object once (when the program runs) and then called the method who converts binary=>string every time. Also, what's the criterion for choosing between a class and a module ? What's the reason for using a "map" right there ? –  RobDel Feb 19 at 9:43
    
I included the example to show how this would look like as a Class. It's not the most efficient, because, really, it's just a very thin wrapper around a function. A class generally carries some sort of state (in an @instance_variable). A module doesn't have state, it only has "class methods" which are basically just "pure" functions mapping some input to some output. –  britishtea Feb 19 at 9:51
    
It's worth mentioning classes have other useful functionality. Typecasting, as demonstrated by 'to_s`, is a very useful feature. Comparison is another. –  Damien Wilson Feb 25 at 21:10

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.