On 02/16/2010 05:58 PM, Len Sumnler wrote:
> Would appreciate is if someone could look over the attached file which
> contains two modules.
> 
> I have a large number of old Cobol files to convert into a MySQL
> database.  The programs work upto this point and I have not added the
> code to move the result into MySQL yet.  It merely prints the hash of
> the binary file so far.
> 
> I am just starting to learn Ruby and OOP as well and am more concerned
> that I am bringing to much of my procedural coding style into my OOP
> programs.
> 
> Comments, suggestions and advice welcomed.

I believe it looks pretty good already.  I think you have grokked OO 
basics, although you could have more classes - something which often 
happens when starting with OO.  In your case I would add these classes

RecordField = Struct.new :name, :type, :length, :decimal

aropnfil = [
   RecordField['cust_no', :Xstr:, 6, 0],
   RecordField['doc_date', :Date, 6, 0],
...
]

Also I'd use Symbols rather than String for the type qualifiers (or even 
class objects, if that's possible) because they are more efficient than 
String (you typically have only few values and each repeats quite a bit).

Your record_length is redundant if I'm not mistaken (it's the sum of all 
field lengths).  So I'd probably also add a class for the complete 
record description.

RecordDefinition = Struct.new :record_fields do
   def initialize(fields = [])
     self.record_fields = fields
   end

   def record_length
     record_fields.inject(0) {|sum, rd| sum + rd.length}
   end
end


aropnfil = RecordDefinition.new [
   RecordField['cust_no', :Xstr:, 6, 0],
   RecordField['doc_date', :Date, 6, 0],
...
]


I would then model WangFile's interface similar to File and include 
Enumerable

def WangFile

   def self.foreach(*a, &b)
     wf = open(*a)
     begin
       wf.each_record(&b)
     ensure
       wf.close
     end
   end

   def self.open(*a)
     new(*a)
   end

   def close
     ...
   end

   def initialize(file_name, record_definition)
     ...
   end

   include Enumerable

   def each_record
     len = record_definition.record_length

     while buffer = ....read(len)
       yield(parse_record(buffer))
     end

     self
   end

   alias each each_record
end

And a minor detail: you can save typing by doing

hash_record[name] = case type
when :Xstr
   @record[start,len]
when :Xint
   @record...

Kind regards

	robert


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