2
\$\begingroup\$

I am writing Python code which reads through two text files and prints the output based on those two files. Basically, the first file, demo.txt, has a matrix of numbers and the second file, days.txt, has a value. Based on that value, many digits in the first file are replaced with the $, ?, or just a space.

Anyways, my code is working perfectly, but I would like to simplify it in a way that it is easier to understand for very new programmers. So you can break it into many parts, change the list comprehensions on line 17. Also, can we break up the with statement on line 12 as well?

def repl(ch,threshold):
    if ch.isdigit():
        if int(ch) < threshold:
            return '$'
        elif int(ch)> threshold:
            return " "
        else:
            return ch
    else:
        return '?'
lines = []
with open('data') as f, open("data.txt","r") as f2:
    threshold = int(f2.read().split('=')[-1])
    for line in f:
        line = line.strip()
        line = line.split()
        line = [repl(ch,threshold) for ch in line]
        lines.append(line)
    # reset to start of demo.txt
    f.seek(0)
    for line in f:
        print(line)

for line in lines:
    print()
    for el in line:
        print(el,end=" ")
\$\endgroup\$
1
\$\begingroup\$

I’d go with:

def replace(word, threshold):
    """Convert any string ('word') that does not equal 'threshold'
    into: '?' if it is not an integer, '$' if it is lower than
    'threshold', or ' ' if it is greater than 'threshold'.
    """
    try:
        value = int(word)
    except ValueError:
        return '?'
    if value < threshold:
        return '$'
    if value > threshold:
        return ' '
    return word

def get_threshold(filename):
    """Read the first line of 'filename', split it around an '=' sign
    and convert the last part into an integer.
    """
    with open(filename) as f:
        return int(f.readline().split('=')[-1])

def process_file(data_file, threshold_file):
    """Replace data from 'data_file' according to the value found in
    'threshold_file'. Print the original data and then the converted
    one.
    """
    threshold = get_threshold(threshold_file)

    lines = []
    print('Original data:')
    with open(data_file) as f:
        for line in f:
            line = line.strip()
            print(line)

            # Join back the converted bits into a single line
            replaced_line = ' '.join(
                # Split the line at words boundaries and convert each
                # bit of text according to the threshold
                replace(chunck, threshold) for chunck in line.split())
            lines.append(replaced_line)

    print('\nData replaced with threshold', threshold)
    for line in lines:
        print(line)

# Does not execute anything unless run from the command line.
if __name__ == '__main__':
    # Call the main function of the program
    process_file('data', 'data.txt')

The key is to have simple functions that one can easily test in the interpreter and extensive documentation for trickier parts.

Docstrings should be enough to mostly describe what is going on; adapt to the intended audience. Using descriptive variable names can also improve the understanding.

For the processing of the data file, there are 3 possibilities:

  1.  with open(data_file) as f:
         lines = [' '.join(replace(chunck, threshold) for chunck in line.strip().split())
             for line in f if not print(line)]
    

    Pretty dense and it uses a test only for its side effect in order to avoid an f.seek(0) and an other iteration over the file. Not ideal. Definitely not the right choice for the purpose.

  2. lines = []
    with open(data_file) as f:
        for line in f:
            line = line.strip()
            print(line)
    
            lines.append(' '.join(replace(chunck, threshold) for chunck in line.split()))
    

    Easier to read: an explicit loop reduce the complexity to decode the list-comprehension. It also allows to clean the lines before printing them and we still avoid an unnecessary f.seek(0). However, there is still a list-comprehension that might be hard to read (even with the comments) depending on your audience.

  3. lines = []
    with open(data_file) as f:
        for line in f:
            line = line.strip()
            print(line)
    
            converted = []
            for chunck in line.split():
                converted.append(replace(chunck, threshold))
            lines.append(' '.join(converted))
    

    Each line is now easily understandable on its own.

I however prefer the second version since it is understandable and teaches good practices without being too aggressive. (the use of a generator expression is recommended here since we don't need the intermediate list.)

Talking about good practices:

  • using join is definitely better than the double for loop you use at the end of your code;
  • the if __name__ == '__main__': construct is recommended but can be confusing, remove it if you think it will be too much for your audience.
\$\endgroup\$
  • \$\begingroup\$ Thank you! I have another question. Instead of print('\nData replaced with threshold', threshold) we need to print the entire contents of the second text file. \$\endgroup\$ – Starry Night Oct 30 '15 at 15:47
  • \$\begingroup\$ @StarryNight Which second file? The one you opened as f2 in your code? You’re not printing it anywhere. \$\endgroup\$ – Mathias Ettinger Oct 30 '15 at 16:17
  • \$\begingroup\$ Yeah but I want to print it along with the first file. Can we make separate functions to open and print both the text files and print them at the bottom? \$\endgroup\$ – Starry Night Oct 30 '15 at 16:18
  • \$\begingroup\$ @StarryNight I’d go with modifying get_threshold if you don't mind having it printed before the data. \$\endgroup\$ – Mathias Ettinger Oct 30 '15 at 16:20
  • \$\begingroup\$ I am able to print both on my own, but I think it would look very neat if they are divided into two separate functions. BTW, I love how you have simplified the code. Can you please implement those in your code as well? \$\endgroup\$ – Starry Night Oct 30 '15 at 16:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.