Asp Forum - Ruby programming styles and new program?

Bill

10/11/2004 12:03:00 AM

Hello,
As a newbie to Ruby, I've just written a program that uses Ruby for
caller ID services on a WinXP machine. It seems to work okay, but I come
from C/++, and wonder if I really did the program the way Ruby is
generally done. Any suggestions on making this more Ruby-like? In
particular, I'm still unsure about variable scoping rules. Does anyone
have a link to a document that explains Ruby variable scoping well?
Also, in Java or C#, the program logic is generally wrapped in a
class-is this also the way things are done in ruby, or is procedural
logic okay?

=========== cut ===========
#!/usr/bin/ruby -w

#########################################################
#
# log_CID.rb
#
# Ruby CID logging script
#
# Logs all activity at a modem set to decipher CID
# (Caller ID) information. Backs up weekly also.
#
##########################################################

require 'date'
require 'zip/zip'
require 'serialport/serialport.so'

############################################
#
# local settings here.
#
$backup_zip_filename = "CID_Data.zip"
modem_init_string = "AT+VCID=1\r\n" # for USR verbose CID output
backup_dir = "c:/modemlog"
base_log_name = "CID"
wday_to_backup = 7
hr_to_backup = 2
port_name = 'COM3:'
days_before_archive = 7
# const for backup intervals
ARCHIVE_DAYS_SECS = 60 * 60 * 24 * days_before_archive
# debug on or off?
DEBUG = true
#
#
##############################################

# var for calendar based backup, start with invalid value.
last_backup_day = 367

#################################
# subroutines

def get_current_fname(backup_dir, base_log_name)
t = Time.now
fname = backup_dir + '/' + base_log_name +
t.strftime("%Y%m%d") + ".log"
return fname
end

def archive_old_to_zip(backup_dir, base_log_name, t)
moved = 0
Dir.foreach(backup_dir) {
| logfile |
if(logfile =~ /^CID/)
ftime = File.stat(logfile).mtime
if(t > ftime + ARCHIVE_DAYS_SECS)
moved += 1 if move_to_archive(logfile)
end
end
}
return moved
end

def move_to_archive(fname)
Zip::ZipFile.open($backup_zip_filename, 1) {
| zfile |
zfile.add(fname, fname)
}
File.delete(fname)
end

def log_text(bckup_dir, bse_log_name, txt)
log_name = get_current_fname(bckup_dir, bse_log_name)
logfile = File.new(log_name, "a")
logfile.print(txt)
logfile.close
end

###############################
# begin main program

# move to the dir for backup
Dir.chdir(backup_dir)

# Open the port and set up CID via modem_init_string.
port = SerialPort.new(port_name)
port.read_timeout = 0
port.puts(modem_init_string)

print "Starting run with port ", port_name,
" and logging to dir ", backup_dir, "\n" if DEBUG

# Loop with pauses to look for data at the port we can record.
while(true)
while(text = port.gets)
print text if DEBUG
# squeeze double \r, etc to just \n
text.sub!(/[\r\n]+/, "\n")
# log text unless it is just spaces
if(text =~ /\S/)
log_text(backup_dir, base_log_name, text)
# archive old logs daily
t = Time.now
yday = t.yday
if(yday != last_backup_day)
archive_old_to_zip(backup_dir, base_log_name, t)
last_backup_day = yday
end
end
end
if(DEBUG)
msg = Time.now.to_s +
": dropped out of system call, restarting loop.\n"
print msg
log_text(backup_dir, base_log_name, msg)
end
end

return 0

========================================

23 Answers

Eivind Eklund

10/11/2004 9:14:00 AM

On Mon, 11 Oct 2004 09:04:40 +0900, Bill <bi11@lynxview.com> wrote:
> Hello,
> As a newbie to Ruby, I've just written a program that uses Ruby for
> caller ID services on a WinXP machine. It seems to work okay, but I come
> from C/++, and wonder if I really did the program the way Ruby is
> generally done. Any suggestions on making this more Ruby-like? In
> particular, I'm still unsure about variable scoping rules. Does anyone
> have a link to a document that explains Ruby variable scoping well?
> Also, in Java or C#, the program logic is generally wrapped in a
> class-is this also the way things are done in ruby, or is procedural
> logic okay?
>
> =========== cut ===========
> #!/usr/bin/ruby -w
>
> #########################################################
> #
> # log_CID.rb
> #
> # Ruby CID logging script
> #
> # Logs all activity at a modem set to decipher CID
> # (Caller ID) information. Backs up weekly also.
> #
> ##########################################################
>
> require 'date'
> require 'zip/zip'
> require 'serialport/serialport.so'
>
> ############################################
> #
> # local settings here.
> #
> $backup_zip_filename = "CID_Data.zip"
> modem_init_string = "AT+VCID=1\r\n" # for USR verbose CID output
> backup_dir = "c:/modemlog"
> base_log_name = "CID"
> wday_to_backup = 7
> hr_to_backup = 2
> port_name = 'COM3:'
> days_before_archive = 7
> # const for backup intervals
> ARCHIVE_DAYS_SECS = 60 * 60 * 24 * days_before_archive
> # debug on or off?
> DEBUG = true
> #
> #
> ##############################################
>
> # var for calendar based backup, start with invalid value.
> last_backup_day = 367
>
> #################################
> # subroutines

class BackupClass
attr_reader :backup_dir, :base_log_name
def initialize(backup_dir, base_log_name)
@backup_dir = backup_dir
@base_log_name = base_log_name
end

>
> def get_current_fname(backup_dir, base_log_name)
> t = Time.now
> fname = backup_dir + '/' + base_log_name +
> t.strftime("%Y%m%d") + ".log"
> return fname
> end

def current_fname
"#{backup_dir}/#{base_log_name}#{Time.now.strftime("%Y%m%d")}.log"
end

> def archive_old_to_zip(backup_dir, base_log_name, t)
> moved = 0
> Dir.foreach(backup_dir) {
> | logfile |
> if(logfile =~ /^CID/)
> ftime = File.stat(logfile).mtime
> if(t > ftime + ARCHIVE_DAYS_SECS)
> moved += 1 if move_to_archive(logfile)
> end
> end
> }
> return moved
> end

#
# I'm not entirely sure about the inject rewrite here; just
# accumulating on moved may be just as good.
#
def archive_old_to_zip(backup_dir, base_log_name, t)
dir = Dir.open(backup_dir)
moved = dir.inject(0) do |lcount, ogfile|
# Skip-logic can use trailing conditionals
next unless logfile =~ /^CID/
next unless t > File.stat(logfile).mtime + ARCHIVE_DAYS_SECS
# Active logic use prefix if
if (move_to_archive(logfile))
next count + 1
else
next count
end
end
dir.close
return moved
end

# I don't like the $backup_zip_filename below; should come from
# the object.
> def move_to_archive(fname)
> Zip::ZipFile.open($backup_zip_filename, 1) {
> | zfile |
> zfile.add(fname, fname)
> }
> File.delete(fname)
> end

> def log_text(bckup_dir, bse_log_name, txt)
> log_name = get_current_fname(bckup_dir, bse_log_name)
> logfile = File.new(log_name, "a")
> logfile.print(txt)
> logfile.close
> end

def log_text(txt)
logfile = File.new(current_fname, "a")
logfile.print(txt)
logfile.close
end

# ... and continue refactoring into an object.

I can do another pass if you finish the refactoring job.

Eivind.

Bill

10/12/2004 5:13:00 AM

Eivind Eklund wrote:
>
> #
> # I'm not entirely sure about the inject rewrite here; just
> # accumulating on moved may be just as good.
> #
> def archive_old_to_zip(backup_dir, base_log_name, t)
> dir = Dir.open(backup_dir)
> moved = dir.inject(0) do |lcount, ogfile|
> # Skip-logic can use trailing conditionals
> next unless logfile =~ /^CID/
> next unless t > File.stat(logfile).mtime + ARCHIVE_DAYS_SECS
> # Active logic use prefix if
> if (move_to_archive(logfile))
> next count + 1
> else
> next count
> end
> end
> dir.close
> return moved
> end
>

Thanks for the suggestion to put the logging/archiving code in a class.
That refactoring does allow more appropriate data scoping.

One problem: I cannot find documentation for the Dir.inject method. I
assume from some of the Ruby docs this is like the Smalltalk inject
method, but I don't know why it's not documented in the Dir class?

Anyway, what, exactly, is happening in the line (typo removed)

> moved = dir.inject(0) do |count, logfile|

and how do you know to put the | count, logfile | variables in that
order, and not | logfile, count | ?

Robo

10/12/2004 5:38:00 AM

Bill wrote:
> One problem: I cannot find documentation for the Dir.inject method. I
> assume from some of the Ruby docs this is like the Smalltalk inject
> method, but I don't know why it's not documented in the Dir class?
>

The Dir class includes the Enumerable module (like most classes that has
a 'each' method), and that module has the inject method.

> Anyway, what, exactly, is happening in the line (typo removed)
>
> > moved = dir.inject(0) do |count, logfile|
>
> and how do you know to put the | count, logfile | variables in that
> order, and not | logfile, count | ?
>

Go to the doc for Enumerable#inject and you'll see the description for it.

Robo

Markus

10/12/2004 5:47:00 AM

On Mon, 2004-10-11 at 22:14, Bill wrote:
> One problem: I cannot find documentation for the Dir.inject method. I
> assume from some of the Ruby docs this is like the Smalltalk inject
> method, but I don't know why it's not documented in the Dir class?

Yes, very much like.
It is a mix-in, from the module Enumerable; thus anything that has
an each & includes Enumerable (e.g. arrays, files, etc.) supports
inject. Rather than documenting it for each of these most references
just list the "mix-ins" of a class as a reminder that you are getting
some extra goodies for free.

> Anyway, what, exactly, is happening in the line (typo removed)
>
> > moved = dir.inject(0) do |count, logfile|
>
> and how do you know to put the | count, logfile | variables in that
> order, and not | logfile, count | ?

Inject works on the model of "injecting" an operator between each
element of an Enumerable collection, prefaced with a starting value.
Thus:

[2,5,7,4].inject(0) { |running_total,x| running_total + x }

means:

0 + 2 + 5 + 7 + 4

or (in case order of evaluation matters to you):

(((0 + 2) + 5) + 7) + 4

just like (IIRC) in smalltalk.

-- Markus

Bill

10/13/2004 2:01:00 AM

Eivind Eklund wrote:
> # ... and continue refactoring into an object.
>
> I can do another pass if you finish the refactoring job.
>
> Eivind.
>
>

Thanks. Okay, here's refactoring pass 1:

=============================================

#!/usr/bin/ruby -w
#########################################################
#
# log_CID.rb
#
# Ruby CID logging script
#
# Logs all activity at a modem set to decipher CID
# (Caller ID) information. Archives the daily logs as well.
#
##########################################################

require 'zip/zip'
require 'serialport/serialport.so'

############################################
#
# local settings here.
#
# name of archive file--daily logs are moved to this archive
backup_zip_filename = "CID_Data.zip"
# modem initialization string.
# need to set to log verbose caller ID information (+VCID=1 or #CID=1, etc)
# also need to set to NOT answer, just monitor line (usually the default)
modem_init_string = "AT+VCID=1\r\n" # for USR verbose CID output
# directory to kep log files
backup_dir = "c:/modemlog"
# base log name for daily log files
# daily log file name is this, plus YYYYMMDD date, plus .log extension
# eg. CID20041004.log
base_log_name = "CID"
# the comm port having the CID-capable modem
port_name = 'COM3:'
# days that a daily log file is kept prior to archiving the file
days_before_archive = 7
# maximum port read errors allowed before aborting run
MAX_PORT_ERRORS = 3000
# debug on or off?
DEBUG = true
#
#
##############################################

#################################

# script local class

class DailyLogWithArchive
attr_reader :backup_dir, :base_log_name, :backup_zip_filename,
:archive_days_secs, :backup_days_interval

def initialize(backup_dir, base_log_name, backup_zip_filename,
days_before_backup, backup_days_interval = 1)
@backup_dir = backup_dir
@base_log_name = base_log_name
@backup_zip_filename = backup_zip_filename
@archive_days_secs = 60 * 60 * 24 * days_before_backup
@backup_days_interval = backup_days_interval
# var for calendar based backup, start with invalid value.
@last_backup_day = -1
end

def current_fname
"#{backup_dir}/#{base_log_name}#{Time.now.strftime("%Y%m%d")}.log"
end

def archive_old_to_zip
time = Time.now
dir = Dir.open(backup_dir)
moved = dir.inject(0) do | move_count, logfile |
next unless logfile.index(base_log_name) == 0
next unless time > File.stat(logfile).mtime + archive_days_secs
if(move_to_archive(logfile))
next move_count + 1
else
next move_count
end
end
dir.close
return moved
end

def move_to_archive(fname)
Zip::ZipFile.open(backup_zip_filename, 1) {
| zfile |
zfile.add(fname, fname)
}
File.delete(fname)
end

def log_text(txt)
logfile = File.new(current_fname, "a")
logfile.print(txt)
logfile.close
# archive old logs daily
time = Time.now
yday = time.yday
if(yday != last_backup_day and
(yday >= last_backup_day + backup_days_interval or yday == 0) )
archive_old_to_zip
last_backup_day = yday
end
end

end

###############################
# begin main program

# var to hold port read error count
port_err_count = 0

# move to the dir for backup
Dir.chdir(backup_dir)

# Open the port and set up CID via modem_init_string.
port = SerialPort.new(port_name)
# indefinite wait for a string to appear at the port
port.read_timeout = 0
port.puts(modem_init_string)

print "Starting run with port ", port_name,
" and logging to dir ", backup_dir, "\n" if DEBUG

# set up the logging class
logger =
DailyLogWithArchive.new(backup_dir, base_log_name, backup_zip_filename)

# Loop with pauses to look for data at the port we can record.
while(true)
while(text = port.gets)
print text if DEBUG
# log text unless it is just spaces
if(text =~ /\S/)
# squeeze double \r, etc to just \n
text.sub!(/[\r\n]+/, "\n")
logger.log_text(text)
end
end
msg = "#{Time.now.to_s}: dropped out of system call, restarting
loop.\n"
print msg if DEBUG
logger.log_text(msg) if DEBUG
port_err_count += 1
if(port_err_count > MAX_PORT_ERRORS)
msg = "Too many port errors...exiting\n"
print msg
logger.log_text(msg)
return port_err_count
end
end

return 0

===========================================

Eivind Eklund

10/13/2004 2:30:00 PM

On Wed, 13 Oct 2004 11:04:45 +0900, Bill <wherrera@lynxview.com> wrote:
> Thanks. Okay, here's refactoring pass 1:
>
> ############################################
> #
> # local settings here.
> #
> # name of archive file--daily logs are moved to this archive
> backup_zip_filename = "CID_Data.zip"
> # modem initialization string.
> # need to set to log verbose caller ID information (+VCID=1 or #CID=1, etc)
> # also need to set to NOT answer, just monitor line (usually the default)
> modem_init_string = "AT+VCID=1\r\n" # for USR verbose CID output
> # directory to kep log files
> backup_dir = "c:/modemlog"
> # base log name for daily log files
> # daily log file name is this, plus YYYYMMDD date, plus .log extension
> # eg. CID20041004.log
> base_log_name = "CID"
> # the comm port having the CID-capable modem
> port_name = 'COM3:'
> # days that a daily log file is kept prior to archiving the file
> days_before_archive = 7
> # maximum port read errors allowed before aborting run
> MAX_PORT_ERRORS = 3000
> # debug on or off?
> DEBUG = true
> #
> #
> ##############################################

All of these "lonely constants and variables" seems ... wrong.
Wouldn't they be better off as methods on an object?

The class refactoring looked OK, except that I would line up the right
hand side of the assignments in initialize.

> ###############################
> # begin main program
>
> # var to hold port read error count
> port_err_count = 0
>
> # move to the dir for backup
> Dir.chdir(backup_dir)
>
> # Open the port and set up CID via modem_init_string.
> port = SerialPort.new(port_name)
> # indefinite wait for a string to appear at the port
> port.read_timeout = 0
> port.puts(modem_init_string)
>
> print "Starting run with port ", port_name,
> " and logging to dir ", backup_dir, "\n" if DEBUG
>
> # set up the logging class
> logger =
> DailyLogWithArchive.new(backup_dir, base_log_name, backup_zip_filename)
>
> # Loop with pauses to look for data at the port we can record.
> while(true)

Use loop do ... end instead of white(true)

> while(text = port.gets)

Use port.each_line do |text|

> print text if DEBUG
> # log text unless it is just spaces
> if(text =~ /\S/)

I'd turn this on it's head: next if text =~ /^\s*$/

> # squeeze double \r, etc to just \n
> text.sub!(/[\r\n]+/, "\n")
> logger.log_text(text)
> end
> end
> msg = "#{Time.now.to_s}: dropped out of system call, restarting
> loop.\n"
> print msg if DEBUG
> logger.log_text(msg) if DEBUG

Join out double conditionals, and move msg calculation under them.

> port_err_count += 1
> if(port_err_count > MAX_PORT_ERRORS)
> msg = "Too many port errors...exiting\n"
> print msg
> logger.log_text(msg)
> return port_err_count
> end
> end
>
> return 0

The logger.log_text(msg) / print msg duplication should be joined up
into a single method.

I've got a feeling most of the main loop and constants could, with
benefit, be refactored into a method object.

Eivind.

Bill

10/14/2004 4:28:00 AM

Eivind Eklund wrote:
>
> All of these "lonely constants and variables" seems ... wrong.
> Wouldn't they be better off as methods on an object?

I guess so. That would, I guess, mean creating a ModemMonitor class
(derived from a MonitorServiceClass?), but you'd still need to put the
initialization values somewhere where they could be edited for script
application on different systems.

Has anyone created a YAML.rb derived class for config files? That would
pull those 'lonely' variables out into a separate file?

>
> I've got a feeling most of the main loop and constants could, with
> benefit, be refactored into a method object.
>

Ara.T.Howard

10/14/2004 5:24:00 AM

Eivind Eklund

10/14/2004 11:54:00 AM

On Thu, 14 Oct 2004 13:29:32 +0900, Bill <bi11@lynxview.com> wrote:
> Eivind Eklund wrote:
> >
> > All of these "lonely constants and variables" seems ... wrong.
> > Wouldn't they be better off as methods on an object?
>
> I guess so. That would, I guess, mean creating a ModemMonitor class
> (derived from a MonitorServiceClass?), but you'd still need to put the
> initialization values somewhere where they could be edited for script
> application on different systems.
>
> Has anyone created a YAML.rb derived class for config files? That would
> pull those 'lonely' variables out into a separate file?

I've done this before by just putting the variables in a single Hash,
with a gating object around it, then I used YAML to load / save the
hash. That worked well for me.

Eivind.

Bill

10/15/2004 4:57:00 AM

Here's the (nearly last) refactoring:
(text should word wrap at 80, but the news program wants to wrap earlier---)

#!/usr/bin/ruby -w
#########################################################
#
# log_CID.rb
#
# Ruby CID logging script
#
# Logs all activity at a modem set to decipher CID
# (Caller ID) information. Archives the daily logs as well.
#
##########################################################

require 'zip/zip'
require 'serialport/serialport.so'
require 'yaml'

# name of config file (YAML format)
config_yaml = 'log_CID.yml'

#################################

# script local classes

class ModemCIDMonitor
attr_reader :port_err_count, :config, :log_blank_lines
attr :debug, :port_err_count

def initialize(config_hash, logger)
@port_err_count = config_hash['port_err_count'] || 0
@port_name = config_hash['port_name'] ||
'COM1:'
@port = SerialPort.new(@port_name)
@MAX_PORT_ERRORS = config_hash['MAX_PORT_ERRORS'] || 100
@debug = config_hash['DEBUG'] ||
false
@log_blank_lines = config_hash['log_blank_lines'] ||
false
@logger = logger
@port.read_timeout = 0
@port_err_count = 0
@modem_init_string =
config_hash['modem_init_string'] || "AT+VCID=1\r\n"
@port.puts(@modem_init_string)
end

def log(txt)
print txt if debug
@logger.log_text(txt)
end

def run
print "Starting run with port ", @port_name,
" and logging to dir ", @logger.archive_dir, "\n"
loop do
@port.each_line do | text |
# log text unless it is just spaces
next unless text =~ /\S/ or log_blank_lines
# squeeze double \r, etc to just \n
text.sub!(/[\r\n]+/, "\n")
log(text)
end
msg =
"#{Time.now.to_s}: dropped out of system call, restarting
loop.\n"
log(msg) if debug
@port_err_count += 1
if(@port_err_count > MAX_PORT_ERRORS)
errmsg = "Too many port errors...ending run\n"
log(errmsg)
return errmsg
end
end
end
end

class CID_Config
attr_reader :config_file, :as_hash

def initialize(config_file)
@config_file = config_file
@as_hash = YAML::load( File.open(@config_file))
end
end

class DailyLogWithArchive
attr_reader :archive_dir, :base_log_name, :archive_zip_filename,
:archive_days_secs, :archive_days_interval
attr :debug

def initialize(config_hash)
@archive_dir = config_hash['archive_dir']
|| './'
@base_log_name = config_hash['base_log_name']
|| 'CID'
@debug = config_hash['DEBUG']
|| false
@archive_days_interval = config_hash['archive_days_interval']
|| 1
@archive_zip_filename = config_hash['archive_zip_filename'] ||
'CID_archive.zip'
@last_archive_day = -1
@archive_days_secs = 60 * 60 * 24 * 7
@archive_days_secs =
60 * 60 * 24 * config_hash['days_before_archive'] if
(config_hash['days_before_archive'])
end

def current_fname
"#{archive_dir}/#{base_log_name}#{Time.now.strftime("%Y%m%d")}.log"
end

def archive_old_to_zip
time = Time.now
wd = Dir.getwd
Dir.chdir(@archive_dir)
dir = Dir.open(@archive_dir)
moved = dir.inject(0) do | move_count, logfile |
next unless logfile
next unless logfile =~ /^#{base_log_name}/
next unless time > File.stat(logfile).mtime +
@archive_days_secs
if(move_to_archive(logfile))
log_text("LOGGER: Archiving file " + logfile + "\n") if
@debug
next move_count + 1
else
next move_count
end
end
dir.close
Dir.chdir(wd)
return moved
end

def move_to_archive(fname)
Zip::ZipFile.open(@archive_zip_filename, 1) {
| zfile |
return nil if zfile.exist?(fname)
zfile.add(fname, fname)
}
File.delete(fname)
end

def log_text(txt)
wd = Dir.getwd
Dir.chdir(@archive_dir)
logfile = File.new(current_fname, "a")
logfile.print(txt)
logfile.close
# check if we should do periodic archiving (up to daily)
time = Time.now
yday = time.yday
if(yday != @last_archive_day and
(yday >= @last_archive_day + @archive_days_interval or yday
== 0) )
archive_old_to_zip
@last_archive_day = yday
end
Dir.chdir(wd)
end
end

###############################
# begin main program

# get config from YAML text file
config = CID_Config.new(config_yaml)

# set up the logging class
logger = DailyLogWithArchive.new(config.as_hash)

# set up modem monitor
monitor = ModemCIDMonitor.new(config.as_hash, logger)

# run -- no return from this unless abort on port timeouts or errors
errmsg = monitor.run

print errmsg

return 0

=================================================

log_CID.yml file:

############################################
#
# log_CID YAML config file.
#

# name of archive file--daily logs are moved to this archive
archive_zip_filename: CID_Data.zip

# modem initialization string.
# need to set to log verbose caller ID information (+VCID=1 or #CID=1, etc)
# also need to set to NOT answer, just monitor line (usually the default)
modem_init_string: AT+VCID=1\r\n

# directory to keep log files
archive_dir: c:/USR/log_CID

# base log name for daily log files
# daily log file name is this, plus YYYYMMDD date, plus .log extension
# eg. CID20041004.log
base_log_name: CID

# the comm port having the CID-capable modem
port_name: 'COM3:'

# days that a daily log file is kept prior to archiving the file
days_before_archive: 7

# maximum port read errors allowed before aborting run
MAX_PORT_ERRORS: 3000

# whether to log whitespace-only lines
log_blank_lines: false

# debug on or off?
DEBUG: true

#
# end YAML config file.
#
#####################################################

comp.lang.ruby

Ruby programming styles and new program?

Bill

Eivind Eklund

Bill

Robo

Markus

Bill

Eivind Eklund

Bill

Ara.T.Howard

Eivind Eklund

Bill

x Login to ForumsZone