breaking from loop

[bruno at modulix]

Ritesh Raj Sarraf wrote:
> Hi,
> 
> Following is the code:
> 
> def walk_tree_copy(sRepository, sFile, sSourceDir, bFound = None):
>     try:
>         if sRepository is not None:

You're being overly defensive here. Passing None as first arg is clearly
a programming error, so the sooner you detect it the better. Hence, just
don't test, and let exceptions propagate.

>             for name in os.listdir(sRepository):
>                 path = os.path.join(sRepository, name)
>                 if os.path.isdir(path):
>                     walk_tree_copy(path, sFile, sSourceDir, bFound)
>                 elif name.endswith('.foo') or name.endswith('.bar'):
>                     if name == sFile:

Why do you *first* test on the extension, and *then* on the whole name ?
The boolean expression:
(name.endswith('.foo') or name.endswith('.bar')) and name == sFile
logically implies that :
sFile.endswith('.foo') or sFile.endswith('.bar')

So the first test is pretty useless...

Also, why hardcode such tests ? Python makes it easy to write generic
(hence reusable) code.


>                         try:
>                             shutil.copy(path, sSourceDir)
>                         except IOError, (errno, errstring):
>                             errfunc(errno, errstring)

??? What is this function doing ?

>                         except shutil.Error:
>                             print name + " is available in " +
> sSourceDir + "Skipping Copy!"

Don't assume the cause of the exception. FWIW, try the following snippet:

>>> f = open('test.txt', 'w')
>>> f.write('coucou')
>>> f.close()
>>> for i in range(10):
...     shutil.copy('test.txt', 'testcopy.txt')
...
>>>

Also, stdout is meant to be used for normal program outputs. Other
messages should go to stderr.


>                         bFound = True
>                         break
>                         return bFound

Err... this last statement won't be executed.

>     except OSError, (errno, strerror):
>         print errno, strerror
>
> This function allows me to walk into a directory based tree and search
> for files ending with .foo and .bar. My requirement is to copy
> .foo/.bar.
> Say in directory temp=>test=>test.bar is found. So shutil.copy will
> copy it. I want that once the copy is done, it should make bFound =
> True and get out.

What's the use of setting bFound to True ? If you want to get out
returning a value, just return that value :

# dummy exemple
def test(seq, target):
  for item in seq:
    if item == target:
      return True
  return False

> But since test directory is under temp, work_tree_copy makes two calls
> of the same function _but_ break only is able to get out from the inner
> call.
> 

You should first focus on making it work without worrying about error
handling. Ensure that all return path are covered (in your actual
implementation, your function always return None). Use a callback
function for testing if a file should be copied, so your code has a
chance of being reusable when specs evolve. And two hints:
 * exceptions are not only for error handling, they are also useful to
control flow of execution...
 * generators and iterators are great

AFAICT, what you're trying to do here is mainly to
1/ recursively search files matching a given condition in a directory tree
2/ do something with these files.

A solution could be to split your code accordingly:

def find_in_tree(tree_path, match):
  join = os.path.join
  isdir = os.path.isdir
  isfile = os.path.isfile
  for name in os.listdir(tree_path):
    fpath = join(tree_path, name)
      if isdir(fpath):
        for foundpath in find_in_tree(fpath, match)
          yield foundpath
      elif isfile(fpath) and match(fpath):
	yield fpath
  raise StopIteration

def find_and_copy_to(tree_path,
                     dest_path, match,
                     stop_on_first_match=False):
  done = []
  for fpath in find_in_tree(tree_path, match):
    if fpath not in done:
      shutil.copy(fpath, dest_path)
      done.append(fpath)
      if stop_on_first_match:
        break
  return done

match_foo_bar = lambda fpath: \
 fpath.endswith('.foo') or fpath.endswith('.bar')

done = find_and_copy_to(sRepository,
                        sSourceDir,
                        match_foo_bar,
                        stop_on_first_match=True)


NB1 : not tested

NB2: I'm not sure I clearly undestand your specs (you're mixing
functional specs and implementation details in your description of
you're trying to do), so this may not be exactly what you want...

Also some stylistic advices:

1/ hungarian notation is *evil* - and even worse in a dynamically typed
language. Better use explicit names. What's important is not the type of
a variable, but it's role.

2/ string formating is fine

print >> sys.stderr, \
        "%s is available in %s - skipping copy" % (filename, source_dir)


> Is there a better way to implement it ?

Probably. Anyway, the better implementation is usually no
implementation. What about:

find $Repository -name "*.bar" -exec cp {} $SourceDir/ \;

?-)


Well, time to have a coffee anyway...
HTH

--
bruno desthuilliers
python -c "print '@'.join(['.'.join([w[::-1] for w in p.split('.')]) for
p in 'onurb at xiludom.gro'.split('@')])"