# A small refractory problem

**URL:** <https://www.ruby-forum.com/t/a-small-refractory-problem/71841>\
**Category:** Ruby\
**Created:** [February 22, 2006, 3:48pm UTC](https://www.ruby-forum.com/t/a-small-refractory-problem/71841 "2006-02-22T15:48:17Z")\
**Posts on this page:** 8\
**Page:** 1

<div class="post-metadata">

**Author:** ![byrnejb](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/d2c977/32.png) [@byrnejb](https://www.ruby-forum.com/u/byrnejb)\
**Post date:** [February 22, 2006, 3:48pm UTC](https://www.ruby-forum.com/t/a-small-refractory-problem/71841/1 "2006-02-22T15:48:17Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![byrnejb](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/d2c977/32.png) [@byrnejb](https://www.ruby-forum.com/u/byrnejb)\
**Post date:** [February 22, 2006, 3:55pm UTC](https://www.ruby-forum.com/t/a-small-refractory-problem/71841/2 "2006-02-22T15:55:56Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![byrnejb](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/d2c977/32.png) [@byrnejb](https://www.ruby-forum.com/u/byrnejb)\
**Post date:** [February 22, 2006, 4:19pm UTC](https://www.ruby-forum.com/t/a-small-refractory-problem/71841/3 "2006-02-22T16:19:23Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![byrnejb](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/d2c977/32.png) [@byrnejb](https://www.ruby-forum.com/u/byrnejb)\
**Post date:** [February 22, 2006, 5:18pm UTC](https://www.ruby-forum.com/t/a-small-refractory-problem/71841/4 "2006-02-22T17:18:43Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![byrnejb](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/d2c977/32.png) [@byrnejb](https://www.ruby-forum.com/u/byrnejb)\
**Post date:** [February 22, 2006, 5:21pm UTC](https://www.ruby-forum.com/t/a-small-refractory-problem/71841/5 "2006-02-22T17:21:19Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![byrnejb](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/d2c977/32.png) [@byrnejb](https://www.ruby-forum.com/u/byrnejb)\
**Post date:** [February 22, 2006, 6:21pm UTC](https://www.ruby-forum.com/t/a-small-refractory-problem/71841/6 "2006-02-22T18:21:01Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![byrnejb](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/d2c977/32.png) [@byrnejb](https://www.ruby-forum.com/u/byrnejb)\
**Post date:** [February 22, 2006, 6:07pm UTC](https://www.ruby-forum.com/t/a-small-refractory-problem/71841/7 "2006-02-22T18:07:13Z")

</div>

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 🙂

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…

---

<div class="post-metadata">

**Author:** ![byrnejb](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/d2c977/32.png) [@byrnejb](https://www.ruby-forum.com/u/byrnejb)\
**Post date:** [February 22, 2006, 7:01pm UTC](https://www.ruby-forum.com/t/a-small-refractory-problem/71841/8 "2006-02-22T19:01:39Z")

</div>

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](mailto: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