A small refractory problem

ruby-talk
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

···

--
Posted via http://www.ruby-forum.com/.

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

--
Posted via http://www.ruby-forum.com/\.

3

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.

···

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

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.

--
Ross Bamford - rosco@roscopeco.REMOVE.co.uk

4

Ross Bamford 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

···

--
Posted via http://www.ruby-forum.com/\.

5

James Byrne wrote:

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

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

···

--
Posted via http://www.ruby-forum.com/\.

6

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...

···

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

Ross Bamford 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.

--
Ross Bamford - rosco@roscopeco.REMOVE.co.uk

7

Ross Bamford 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.

···

--
Posted via http://www.ruby-forum.com/\.

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 - <byrnejb@harte-lyne.ca>
#
# 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

--
Posted via http://www.ruby-forum.com/.