A small refractory problem


#1

I have this code:

def move(sourcedir,globspec,targetdir,suf=nil)

if sourcedir.kindof?(self.class) then sourcedir = sourcedir.path end
sourcedir = File.expand_path(sourcedir.to_s)
if !self.validpath?(sourcedir) then
  raise ArgumentError, "Source directory invalid", caller
end

if targetdir.kindof?(self.class) then targetdir = targetdir.path end
targetdir = File.expand path(targetdir.to_s)
if !self.validpath?(targetdir) then
  raise ArgumentError, "Target directory invalid", caller
end

I would like reduce the duplicate statements into a single block
iterated over for source and target. But I lack the ruby syntax
knowledge to determine how best to go about this. I would appreciate
some examples of how this could be done using an array or a hash and
possibly employing symbols. source and target can be an object of the
same class or something that can meaningfully converted into a string
containing a directory path.

I must say that the test/unit approach to programming is, if anything,
understated in its effectiveness. I am simply astonished at how easy
that the test first approach makes programming and how simple the
test/unit framework makes testing first work with ruby. For example,
code above was created to deal with a situation that test/unit testing
uncovered and which would have been very difficult to track down had the
code gotten into production.

Regards,
Jim


#2

Please forgive the missing _ in the first example.
The code should be:

def move(sourcedir,globspec,targetdir,suf=nil)

if sourcedir.kind_of?(self.class) then sourcedir = sourcedir.path end
sourcedir = File.expand_path(sourcedir.to_s)
if !self.validpath?(sourcedir) then
  raise ArgumentError, "Source directory invalid", caller
end

if targetdir.kind_of?(self.class) then targetdir = targetdir.path end
targetdir = File.expand_path(targetdir.to_s)
if !self.validpath?(targetdir) then
  raise ArgumentError, "Target directory invalid", caller
end

#3

On Wed, 2006-02-22 at 23:48 +0900, James B. wrote:

if targetdir.kindof?(self.class) then targetdir = targetdir.path end

same class or something that can meaningfully converted into a string
containing a directory path.

Maybe something like:

def move(sourcedir,globspec,targetdir,suf=nil)
[sourcedir,targetdir].each do |dir|
dir = File.expand_path(dir.to_s)
raise ArgumentError, “#{dir} not valid” unless validpath?(sourcedir)
end
end

Not sure how the rest of your code works, but it seems to me you could
have whatever class this is return ‘path’ from to_s, avoiding the check
you were doing (and avoiding having to set vars outside the block).

Hope that helps.


#4

Ross B. wrote:

Maybe something like:

def move(sourcedir,globspec,targetdir,suf=nil)
[sourcedir,targetdir].each do |dir|
dir = File.expand_path(dir.to_s)
raise ArgumentError, “#{dir} not valid” unless validpath?(sourcedir)
end
end

Not sure how the rest of your code works, but it seems to me you could
have whatever class this is return ‘path’ from to_s, avoiding the check
you were doing (and avoiding having to set vars outside the block).

Hope that helps.

Yes, it does help, very much so. The .move method is essentially
private, although not presently declared as such. The intermediate
methods called from outside the instance are .get and .put which, on
reflection, are probably more suitable places to check the object class
of the passed directory arguments. That simplfies the assignments in the
move class as you recommend.

Thank you.

This is fun.

Regards,
Jim


#5

James B. wrote:

That simplfies the assignments in the move class as you recommend.

.move method rather than move class is what I intended to write.


#6

Ross B. wrote:

And that you want to be able to pass strings, or other DirectoryThings,
as the arguments. So if you have:

    class DirectoryThing
      def to_s
        self.path
      end
    end

You can confidently to_s the argument you’re passed (as above), rather
than having to explicitly check what it is - a string will give you
itself, while a DirectoryThing will give you it’s path.

Just a thought…

And a very good one at that. I can see that I have much to learn. This
approach makes many things very simple. Thank you.


#7

On Thu, 2006-02-23 at 01:18 +0900, James B. wrote:

of the passed directory arguments. That simplfies the assignments in the
move class as you recommend.

Well, glad to be able to help :slight_smile:

I would just say though that I was actually advocating not checking
the arguments, but instead going for more of a duck-typing
implementation.

I’m assuming that the class you’re working on looks something like this:

    class DirectoryThing
      def initialize(dir)
        @dir = dir
      end

      def move(sourcedir,globspec,targetdir,suf=nil)
        # .. code above
      end

      def path
        @dir
      end

      # ... etc ...
    end

And that you want to be able to pass strings, or other DirectoryThings,
as the arguments. So if you have:

    class DirectoryThing
      def to_s
        self.path
      end
    end

You can confidently to_s the argument you’re passed (as above), rather
than having to explicitly check what it is - a string will give you
itself, while a DirectoryThing will give you it’s path.

Just a thought…


#8

This is the entire class. The test unit setup follows. If you have the
inclination then I would apprciate some pointers on how to better employ
the test/unit framework. I have found it immensely useful but I am sure
that there are ways that I could better employ it.

Many thanks,
Jim

#-----------------------------------------------------------------------------
#++

mv2dir.rb

version 1.0 - 2006 Feb 16 - James B. Byrne - removed_email_address@domain.invalid

find, move and optionally rename data files that match glob expression

provided.

#–

class Mv2Dir

require ‘fileutils’
require ‘syslog’

#++

Hold a single logging identifier for the entire class.

@@logid = nil

#++

Holds the instance path value.

attr_reader :path

#++

Holds the most recent instance information message.

attr_reader :message

#++

Returns TRUE if instance path exists, FALSE otherwise.

def exist?
self.validpath?(@path)
end

#++

Alias for exist?

def exists?
self.exist?
end

#++

Given a source dir, file globbing string and optional

extension string, order the arguments placing @path as the

target and call move. (See: put, move)

def get(sourcedir,globspec,suf=nil)
self.move(sourcedir.to_s,globspec,@path,suf)
return self
end

#++

A mv2dir instance is essentially a path to a directory

somewhere in the filesystem.

def initialize(path=’./’)
@path = File.expand_path(path.to_s)
@message = nil
self.validpath?(@path)
return self
end

#++

Check for open logging proces, open one if none exists.

Log message to syslog.

def log(*message)
if !@@logid then @@logid = Syslog.open($0) end
if message.size > 0 then
message.each { |m| @@logid.info(m.to_s) }
else
@@logid.info(@message.to_s)
end
return self
end

#++

Do the actual file moves here.

Given source dir, target dir, file globbing spec, and

optional extension; verify the source and target directories,

raising AgrumentError if invalid, and then get all

the files in source that match and move them to target

appending ext if given. (See: get, put)

def move(sourcedir,globspec,targetdir,suf=nil)

[sourcedir,targetdir].each do |dir|
dir = File.expand_path(dir.to_s)
raise ArgumentError, "#{dir} directory invalid", caller unless \
  self.validpath?(dir)
end

Dir.glob(File.join(sourcedir.to_s,globspec.to_s)).each do |ff|
  fo = File.join(targetdir,File.basename(ff)) + suf.to_s
  @message = "Moving #{ff} to #{fo}"
  FileUtils.mv(ff,fo)
  self.log
end

return self

end

#++

Given a target dir, file globbing string and optional

extension string, pass these arguments to move giving

@path as source. (See: get, move)

def put(targetdir,globspec,suf=nil)
self.move(@path,globspec,targetdir.to_s,suf)
return self
end

#++

Check that directory is readable.

def readable?
if !File.readable?(@path) then
@message = “#{@path} is not readable.”
FALSE
else
@message = “#{@path} is readable.”
TRUE
end
end

#++

In the end, a mv2dir instance is a path and a path is a string.

def to_s
self.path
end

#++

Check that directory argument exists as a directory.

def validpath?(path)
if !File.exists?(path) then
@message = “#{path} does not exist on this system.”
FALSE
elsif
!File.directory?(path) then
@message = “#{path} exists but is not a directory.”
FALSE
else
@message = “#{path} is a valid directory.”
TRUE
end
end

#++

Check that directory argument is writable.

def writable?
if !File.writable?(@path) then
@message = “#{@path} is not writeable.”
FALSE
else
@message = “#{@path} is writeable.”
TRUE
end
end

#++

Alias for writable?

def writeable?
self.writable?
end
#–

end # End class Mv2Dir
#----------------------------------------------------------------------------

class Test_Mv2Dir < Test::Unit::TestCase

@@dir_test = ["~/tmp/mv2dir/test/source","~/tmp/mv2dir/test/target"]

def setup
assert(@@dir_test.length == 2,"@@test_dir must contain at exactly 2
paths")
assert(@@dir_test[0] != @@dir_test[1],"@@test_dir entries must be
different")
end

#1. Test that instances test for valid directories and return

TRUE or FALSE as expected.

def test_dir

  # a.  define test directories:
  dir_test = @@dir_test

  # b.  clear them out:
  dir_test.each do |dt|
    dt = File.expand_path(dt.to_s)
    FileUtils.rm_r(dt, :force => TRUE, :secure => FALSE)
  end

  # c.  create one but no others:

  FileUtils.mkdir_p(File.expand_path(dir_test[0].to_s))

  # d.  Test creation of Mv2Dir instances:

  mv2_in = Mv2Dir.new(File.expand_path(dir_test[0].to_s))

  assert_not_nil(mv2_in)
  assert_equal(File.expand_path(dir_test[0].to_s),mv2_in.path)
  assert(mv2_in.exists?, "#{mv2_in.path} should exist")

  mv2_out = Mv2Dir.new(File.expand_path(dir_test[1].to_s))

  assert_not_nil(mv2_out)
  assert_equal(File.expand_path(dir_test[1].to_s),mv2_out.path)
  assert(!mv2_out.exists?, "#{mv2_out.path} should not exist")

  mv2_dflt = Mv2Dir.new()

  assert_not_nil(mv2_dflt)
  assert_equal(File.expand_path("./"),
    mv2_dflt.path, "#{mv2_dflt.path} should == 

#{File.expand_path("./")}")
assert(mv2_dflt.exists?, “#{mv2_dflt.path} must exist”)

  # e.  test that bogus targets fail

  dir_fail = "~/not/a/directory"
  mv2_fail = Mv2Dir.new(dir_fail)

  assert_not_nil(mv2_fail)
  assert_equal(File.expand_path(dir_fail.to_s),mv2_fail.path)
  assert(!mv2_fail.exists?, "#{mv2_fail.path} should not exist")

  assert_raise(ArgumentError) {mv2_fail.get("/tmp",'xyz*')}

  # f.  Test readable? and writeable?

  #   f1. set the mode to r--r----- on test dir.
  FileUtils.chmod(0440,mv2_in.path)
  assert(mv2_in.readable?,"#{mv2_in.path} should be readable")
  assert(!mv2_in.writeable?,"#{mv2_in.path} should not be 

writeable")
assert(!mv2_out.readable?, “#{mv2_out.path} should not be
readable,
does not exist”)

  #   f2. set the mode to -w---w--- on test dir.
  FileUtils.chmod(0220,mv2_in.path)
  assert(!mv2_in.readable?,"#{mv2_in.path} should not be readable")
  assert(mv2_in.writeable?,"#{mv2_in.path} should be writeable")
  assert(!mv2_out.writeable?, "#{mv2_out.path} should not be 

writeable,
does not exist")

  # g.  clear out the test directories:
  dir_test.each do |dt|
    dt = File.expand_path(dt.to_s)
    FileUtils.chmod(0440,dt) rescue Errno::ENOENT
    FileUtils.rm_r(dt, :force => TRUE, :secure => FALSE)
  end

end # end method test_dir

#2. Create test files in test directories and move them about

checking that the glob arguments are adhered to.

def test_pattern

# Populate the test directory array values.
dir_test = @@dir_test

# set a globbing argument string.
mv2_glob = "Mv2DirFile[1-3]"
# convert it to a regexp for testing.
gp = Regexp.new('^' + mv2_glob + '$')

# Populate an array of passing file names:
mv2_fn_pass = [ "Mv2DirFile1",
                "Mv2DirFile2",
                "Mv2DirFile3"
              ]

# Test that they all pass:
mv2_fn_pass.each do |fp|
  assert_match(gp,fp,"#{fp} does not match #{gp}")
end

# Populate an array of failing file names:
mv2_fn_fail = [ "xYzMv2DirFile1",
                "Mv2DirFile4",
                "Mv2DirFile4.aBc"
              ]

# Test that they all fail:
mv2_fn_fail.each do |ff|
  assert_no_match(gp,ff,"#{ff} does not match #{gp}")
end

# make sure that there are no directories out there:
dir_test.each do |dt|
  dt = File.expand_path(dt.to_s)
  FileUtils.chmod(0440,dt) rescue Errno::ENOENT
  FileUtils.rm_r(dt, :force => TRUE, :secure => FALSE)
  assert(!File.exist?(dt),"#{dt} should not exist")
end

# create the directories:

dir_test.each do |dt|
  dt = File.expand_path(dt.to_s)
  FileUtils.mkdir_p(dt)
  assert(File.exist?(dt),"#{dt} should exist")
end

# create the test files:

(mv2_fn_fail + mv2_fn_pass).each do |sf|
  pf = File.join(File.expand_path(dir_test[0].to_s),sf.to_s)
  FileUtils.touch(pf)
  assert(File.exist?(pf),"#{pf} should exist after touch")
end

# instantiate mv2dir objects for source and target.

mv2_in = Mv2Dir.new(File.expand_path(dir_test[0].to_s))

mv2_out = Mv2Dir.new(File.expand_path(dir_test[1].to_s))

# put the source files to target dir without any extensions

mv2_in.put(mv2_out.path,mv2_glob)

# at this point mv2_out.path should contain only the matched
# files and mv2_in.path only the unmatched files.

mv2_fn_pass.each do |fn|
  pf = File.join(File.expand_path(mv2_out.path),fn.to_s)
  assert(File.exist?(pf),"#{pf} should exist after put")
  ff = File.join(File.expand_path(mv2_in.path),fn.to_s)
  assert(!File.exist?(ff),"#{ff} should not exist after put")
end

mv2_fn_fail.each do |fn|
  ff = File.join(File.expand_path(mv2_out.path),fn.to_s)
  assert(!File.exist?(ff),"#{ff} should not exist after put")
  pf = File.join(File.expand_path(mv2_in.path),fn.to_s)
  assert(File.exist?(pf),"#{pf} should exist after put")
end

# get the files back from the target passing a mv2dir instance
# rather than a path.

mv2_in.get(mv2_out,mv2_glob)

# at this point all of the files should be back in the source
# and none of the files should remain in the target.

(mv2_fn_fail + mv2_fn_pass).each do |fn|
  pf = File.join(File.expand_path(mv2_in.path),fn.to_s)
  assert(File.exist?(pf),"#{pf} should exist after get")
  ff = File.join(File.expand_path(mv2_out.path),fn.to_s)
  assert(!File.exist?(ff),"#{ff} should not exist after get")
end

#TODO:

1. Test appending of suffixes.

end # End method test_pattern

end # End Class Test_Mv2Dir