0

Suppose I have a multi-line string which may contain lines consisting of a single file name. I want to print each line in the string, unless the line is a file name (defined here as ending in '.txt'), in which case I want to print each line in that file, unless the line in that file is a file name etc.

My initial approach is to use a helper function as follows:

def deep_print_helper(file_path, line_sep):
    with open(file_path) as f:
        text = f.read()
        return deep_print(text, line_sep)

def deep_print(s, line_sep):
    lines = s.split(line_sep)
    for l in lines:
        if l.endswith('.txt'):
            deep_print_helper(l, line_sep)
        else:
            print(l)

But having to pass line_sep to the helper function only to pass it back again seems inelegant.

So I tried an approach that uses only one function:

def deep_print(line_sep, s='', file_path=''):
    if file_path:
        with open(file_path) as f:
            s = f.read()
    lines = s.split(line_sep)
    for l in lines:
        if l.endswith('.txt'):
            deep_print(line_sep, file_path=l)
        else:
            print(l)

This has an implicit required argument (either s or file_path but not both), but since the user of the function will only use one form (s=) it may not be too kludgey. It also seems a little odd from a user's perspective that line_sep is the first argument.

Which approach is a better design? Is there another approach I should consider?

Craig Burgler
  • 1,749
  • 10
  • 19

3 Answers3

2

Your question should not be related to Pythonic way to achieve this. It is related to the design, independent of language. If you ask me among the two approaches, I'll go with 1. But the better way to achieve is via class having your functions within it. In Python you may do it like:

class DeepPrint(object):
    def __init__(self, file_path):
        DeepPrint._deep_print_helper(file_path)

    @staticmethod
    def _deep_print_helper(file_path):
        with open(file_path) as f:
            return DeepPrint._deep_print(f)

    @staticmethod
    def _deep_print(f):
        for l in f.readlines():
            if l.endswith('.txt'):
                DeepPrint._deep_print_helper(l)
            else:
                print(l)
Moinuddin Quadri
  • 46,825
  • 13
  • 96
  • 126
  • 1
    Good point about this being a general design question. I changed the title and wording to reflect that. – Craig Burgler Aug 23 '16 at 21:22
  • The `__init__` method should have a `string` argument, not a `file_path` argument as per requirement in OP. Also we can eliminate the inelegantness of the `line_sep` method parameters by storing `line_sep` in an instance variable. – Craig Burgler Aug 23 '16 at 22:16
  • I haven't modified your code. I just moved your functions to the class. You may now check it. I have modified it based on the updates by you – Moinuddin Quadri Aug 23 '16 at 22:40
0

To avoid passing the line_sep parameter, you can define the helper function inside the recursive function:

def deep_print(s, line_sep):

    def deep_print_helper(file_path):
        with open(file_path) as f:
            text = f.read()
            return deep_print(text, line_sep)

    lines = s.split(line_sep)
    for l in lines:
        if l.endswith('.txt'):
            deep_print_helper(l)
        else:
            print(l)
Aran-Fey
  • 39,665
  • 11
  • 104
  • 149
  • 1
    Since it is a recursive function, every time `deep_print` is called, it will create a copy of `deep_print_helper` which is not required – Moinuddin Quadri Aug 23 '16 at 20:29
0

Your requirements may not allow for this, but using str.splitlines would make things a little less complicated. In the same vain, is there a reason why the original file is not opened as part of the recursion (i.e., rather than passing a string into deep_print, you could pass in a file_path)? If those two constraints can be lifted, you could do something like the following:

def deep_print(file_path):
    with open(file_path) as f:
        s = f.read()
    for line in [l.strip() for l in s.splitlines()]:
        if line.endswith('.txt'):
            deep_print(line)
        else:
            print(line)
rkersh
  • 4,447
  • 2
  • 22
  • 31
  • `[l.strip() for l in s.splitlines()]` won't return list of lines, instead words in each line. – Moinuddin Quadri Aug 23 '16 at 21:34
  • Also, OP requires custom line separator – Moinuddin Quadri Aug 23 '16 at 21:34
  • 1
    Actually I really like the `s.splitlines()`. It lets me do away with the `line_sep` argument entirely in my application, which is an answer to this SO question: `http://stackoverflow.com/questions/39093525/how-to-join-incorporate-splitted-lines-with-replacing-data-from-a-file-into-the` . Yes it would be easier to pass a file name to `deep_print` but the question has the string argument requirement. – Craig Burgler Aug 23 '16 at 21:46
  • @MoinuddinQuadri `rkersh's` code actually returns a list of lines; remember `s` is a multi-line string in this context – Craig Burgler Aug 23 '16 at 21:49
  • @Craig: My bad. I thought `strip()` as `split()`. It will work fine. If you don't have custom file separator and your file uses the default, this code will work. But still I think you do not need `strip()` as this is not the part of our requirement. – Moinuddin Quadri Aug 23 '16 at 21:54
  • Right, I introduced `line_sep` only so I could `split()` the lines in the string. No `split()` = no `line_sep` = no `inelegantness`. And yes `for line in s.splitlines()` suffices. – Craig Burgler Aug 23 '16 at 21:58
  • I also could have used `lines = s.split(os.linesep)`, which would also obviate the `line_sep` parameter – Craig Burgler Aug 23 '16 at 23:06