5 Replies - 814 Views - Last Post: 12 July 2013 - 12:55 PM Rate Topic: -----

#1 steve.v  Icon User is offline

  • New D.I.C Head

Reputation: 2
  • View blog
  • Posts: 30
  • Joined: 31-October 12

Code review, 2nd Ruby Program, please help...

Posted 11 July 2013 - 08:38 PM

Ok this is a second attempt at turning one of my C++ programs into Ruby. This program is suppose to read a file as input and output a file. This program reads each character and increments an index in an Array. For example if 'a' is found, it will increment the index at Array[0]. In addition the program will check for newline char and will increment a line count variable. At the end of the program, there is a method to print out the text data from the input file and the array of characters and the letters found.

For example:

The data in the file is:

String of text etc, etc, etc.

A = 2
B = 3
C = 1
E = 8

etc....

However I am having trouble with the output of the program. I believe I have most parts of the program correct. The output I am getting is not how it is supposed to look. Please review the code and provide some feedback. I am dumbfounded as to the amount of lines and code I am writing compared to C++, ruby is awesome! Here is what I have so far:


class TextProcessing
  
  attr_accessor :lineCount, :list, :charArray, :output
  
  def initialize
    j = 0
    @lineCount = 0
    @list = ""
    @charArray = [0]
    @output = File.open("outputfile.txt", "w")
    @output.puts "\nThis program is now reading each character count and line count...\n"
    
    #puts "\nPlease enter in the source address for importing... [ex. './text.txt']: "
    #source = gets.chomp
  
    @list = File.read('./data.txt')
    processText
    writeTotal
    @output.close
    
    
  end
  
  def processText
      @list.each_char do |a| 
          copyText(a)
          characterCount(a)
      end
        
    
  end
  
  def copyText(letter)
      puts "Character found: ---#{letter.to_s}---"
      @output << letter
      if letter.to_s == '\n'
          @lineCount += 1
      end
  end
  
  def characterCount(character)
    index = 0
    index = character.upcase.to_i - 'A'.to_i
    if 0 < index && index <= 26
      @charArray[index] += 1
    end
  end
  
  def writeTotal
    index = 0
    @output.puts "\n \n"
    @output.puts "\nThe number of lines: #{@lineCount}\n"
    @charArray.each do |i|
      @output << "\n#{i.chr} : #{i}\n"
    end
    
  end
    
  
end

myText = TextProcessing.new

   


Is This A Good Question/Topic? 0
  • +

Replies To: Code review, 2nd Ruby Program, please help...

#2 Lemur  Icon User is offline

  • Pragmatism over Dogma
  • member icon


Reputation: 1357
  • View blog
  • Posts: 3,424
  • Joined: 28-November 09

Re: Code review, 2nd Ruby Program, please help...

Posted 11 July 2013 - 09:19 PM

It can be even shorter. Remember, you're not programming in C++, you don't HAVE to use classes, especially for something like this.

characters = {}
file = File.read('./data.txt')

count = 0
contents = file.each.inject("") do |content,line|
  line.each_char{ |ch| characters[ch] += 1 }
  count += 1  
  content << line
end

puts "The number of lines is #{count}"
puts "The contents of the file are: #{contents}"
puts "Letters Count:"
characters.each_pair{ |k,v| puts "#{k}: #{v}" }



To explain what I'm doing here, we're using inject which inserts a new variable into the block that is returned, so you can say something like contents = and assign like that.

By using the each_char method on the line we can take each character and add 1 to the hash key of that character.

<< is the concat operator, which adds whatever the line was to the content.

Inject and the Enumerable module are extremely useful. Inject, map, reduce, select, and other such things venture into lisp land which is where ruby gets some of its more interesting features. Blocks are effectively lambda statements, or anonymous functions.

If you can master blocks, procs, lambdas, and closures you'll have a huge edge in Ruby.

Ah, fair warning that I hammered out that code in about 2 minutes, and I don't have a running version of Ruby around at the moment except for on my phone.
Was This Post Helpful? 1
  • +
  • -

#3 steve.v  Icon User is offline

  • New D.I.C Head

Reputation: 2
  • View blog
  • Posts: 30
  • Joined: 31-October 12

Re: Code review, 2nd Ruby Program, please help...

Posted 11 July 2013 - 10:21 PM

Hello Lemur,

Thank you for taking the time to assist me. I am amaze at the amount of code required for this kind of task. You noted a few things which I will look into regarding inject and enumerable modules. I find string manipulating very challenging for me. In addition, I will also look into blocks, procs and lambdas. The code however returned an undefined method for the each method in line 5. I think this can be fixed easily if I knew how. Wonderful structure and elegant code. I wish to learn more and as much as I can. Thank you once again.

Steve.
Was This Post Helpful? 0
  • +
  • -

#4 sepp2k  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 2086
  • View blog
  • Posts: 3,173
  • Joined: 21-June 11

Re: Code review, 2nd Ruby Program, please help...

Posted 12 July 2013 - 06:29 AM

View PostLemur, on 12 July 2013 - 06:19 AM, said:

Remember, you're not programming in C++, you don't HAVE to use classes


You don't have to use classes in C++ either. That is, you don't have to use your own classes. You probably should use built-in classes like fstream and string, but that's obviously true in Ruby as well.

View PostLemur, on 12 July 2013 - 06:19 AM, said:

characters = {}
file = File.read('./data.txt')

count = 0
contents = file.each.inject("") do |content,line|
  line.each_char{ |ch| characters[ch] += 1 }
  count += 1  
  content << line
end

puts "The number of lines is #{count}"
puts "The contents of the file are: #{contents}"
puts "Letters Count:"
characters.each_pair{ |k,v| puts "#{k}: #{v}" }



Some notes:
  • file already contains the contents of the file as one big string, so the contents variable is redundant.
  • Doing enum.each.enumerable_method is redundant - you can just do enum.enumerable_method.
  • In Ruby 1.9 and later strings don't have an each method (and thus aren't enumerable directly anymore), so you should use file.lines.inject instead.
  • By default unassigned values in a hash are nil, not 0, so you can't use +=. You need to use Hash.new(0) to make 0 the default or check for nil before adding.
  • You're using inject to perform 3 jobs: Inject over the lines to create contents (not necessary); iterate over the lines to count the number of lines (you can use count for that); iterate over the chars to count the chars (you could do this more easily by iterating over the chars directly instead of iterating over each line and then over each char).
  • Unlike the OP you count every char, not just letters.


Here's how I'd do it:

contents = File.read("data.txt")
line_count = contents.lines.count

letter_counts = Hash.new(0)
contents.each_char do |char|
  char = char.upcase
  if ('A' .. 'Z').include? char
    letter_counts[char] += 1
  end
end

puts "The number of lines: #{count}"
puts "Letters Count:"
letter_counts.each do |char, count|
  puts "#{char}: #{count}"
end



@Steve: If you do use classes, it is very bad practice to put your entire application logic into your initialize method. The initialize method should initialize your object to be in a usable state. It should not have any side-effects beyond your object. It generally shouldn't do any IO and certainly not interact with the user. Filenames and similar information should be parameters to initialize - hardcoding them makes the class impossible to reuse. If all you do in the main part of your code is to create an instance of your class and you then don't even call a method on that instance, you're doing it wrong.

Also it would be best practice if all your output code would live in the same place - preferably outside of the class. The usual way to use a class is to instantiate it, call methods on the instance that return the information that you want, and then output that information in whatever way you want. The code to calculate the information you need should be run when you request that information - not when you create the object. And the code to output the information should be outside of the class. This way you can use the same class in a console application, a GUI application and a web application without any adjustments to the class.

So using a class, I'd write your code like this (though Lemur is probably right that classes are overkill here):

class TextInfo
  attr_reader :contents

  def initialize(text)
    @contents = text
  end

  def TextInfo.from_file(filename)
    TextInfo.new( File.read(filename) )
  end

  def letter_counts
    # The first time this method is called, call calculate_letter_counts
    # and store the result in the @letter_counts variable. On later calls
    # simply return the contents of that variable.
    @letter_counts ||= calculate_letter_counts
  end

  def calculate_letter_counts
    counts = Hash.new(0)
    @contents.each_char do |char|
      char = char.upcase
      if ('A' .. 'Z').include? char
        letter_counts[char] += 1
      end
    end
    counts
  end

  def number_of_lines
    @number_of_lines ||= @contents.lines.count
  end
end

text_info = TextInfo.from_file("data.txt")
puts "The number of lines: #{ text_info.number_of_lines }"
puts "Letters Count:"
text_info.letter_counts.each do |char, count|
  puts "#{char}: #{count}"
end



PS: The convention in Ruby is to use snake_case for method and variable names, not camelCase.

This post has been edited by sepp2k: 12 July 2013 - 08:16 AM

Was This Post Helpful? 4
  • +
  • -

#5 Lemur  Icon User is offline

  • Pragmatism over Dogma
  • member icon


Reputation: 1357
  • View blog
  • Posts: 3,424
  • Joined: 28-November 09

Re: Code review, 2nd Ruby Program, please help...

Posted 12 July 2013 - 08:24 AM

Huh, you learn something every day. I'll keep that one in mind.
Was This Post Helpful? 0
  • +
  • -

#6 steve.v  Icon User is offline

  • New D.I.C Head

Reputation: 2
  • View blog
  • Posts: 30
  • Joined: 31-October 12

Re: Code review, 2nd Ruby Program, please help...

Posted 12 July 2013 - 12:55 PM

Hello sepp2k

Very insightful reading and a wake up call to my logic and structure of coding. I now know about the bad habits that I have taken up, thank you for noting it well. It will take some time for me to throughly analyze your class example. I find it very elegant but at the moment a bit complex for me to understand everything. Your script example without the class is a lot easier to understand. The ruby forum is a bit quiet here at dream in code, however its an advantage to me that I get instant replies from good contributors like you and lemur. Thanks once again.
Was This Post Helpful? 0
  • +
  • -

Page 1 of 1