6
\$\begingroup\$

I'm trying to read in a few lines of parameters from an textfile and format these paths.

How can I improve this code to work with less lists? I just need the final paths.

import os
config = open("C:\\Users\\config.txt")
content = config.readlines()
removedEnds = []
removedStarts = []
finalPaths = []

for string in content:
    removedEnds.append(string.rstrip())

for string in removedEnds:
    print(string)
    if string.find('='):
        removedStarts.append(string[(string.find('=')+1):])

for string in removedStarts:
    finalPaths.append(string.split("\\"))

for string in finalPaths:
    print(os.path.join(*string))

The textfile looks like this:

Source=C:\Users\workspace\OMDB\WebContent\app
Target=C:\Apache24\htdocs
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Does it hold only one set of Source vs Target? \$\endgroup\$
    – holroy
    Dec 15 '15 at 10:44
  • \$\begingroup\$ What do you consider to be the output of this code? Which variables are considered important, and which are considered temporary? \$\endgroup\$ Dec 15 '15 at 10:53
  • \$\begingroup\$ @holroy Yes only this two lines \$\endgroup\$
    – JohnDizzle
    Dec 15 '15 at 10:56
  • \$\begingroup\$ @200_success the output should be two filepaths, I only need the final two paths at the end \$\endgroup\$
    – JohnDizzle
    Dec 15 '15 at 10:57
3
\$\begingroup\$

I recommend a completely different solution, based on dict comprehensions, which would make the code much more expressive:

import os
import re

with open(r'C:\Users\config.txt') as f:
    paths = dict(
        re.match('([^=]+)=(.*)', line).groups()
        for line in f if '=' in line
    )
    # Canonicalize backslashes into platform-appropriate delimiters
    paths = dict((k, os.path.join(*v.split('\\'))) for k, v in paths.items())

print(paths)

You would then get the output as a nice data structure:

 {'Source': 'C:\\Users\\workspace\\OMDB\\WebContent\\app', 'Target': 'C:\\Apache24\\htdocs'}

Specifically:

  • Always call open() in the context of a with block, to ensure that the file gets closed with no additional effort.
  • Use r'Raw strings' to avoid double backslashes.
  • The regular expression kind of looks like the text you are trying to match. Also, the (.*) capture group will naturally exclude the trailing newline.
\$\endgroup\$
1
  • \$\begingroup\$ Nice answer, but your generator comprehension, dict((k, os.path.join(*v.split('\\'))) for k, v in paths.items()), is better written as a dictionary comprehension{k: os.path.join(*v.split('\\')) for k, v in paths.items()}. \$\endgroup\$
    – Peilonrayz
    Dec 15 '15 at 14:41
5
\$\begingroup\$

Rather than creating a series of lists, you should just iterate over the lines of the file directly. You don't need to maintain state between any two lines – just process them as you get them.

A few idioms you should also note:

  • Use the with open(...) constructions. This ensures the file is always closed, even if the script throws an exception. (Your script never closes the file.)
  • Rather than using if string.find('='), use if '=' in string.
  • Rather than using string[(string.find('=')+1):] to get the portion after the equals sign, I think it's neater and more Pythonic to use string.split('=', maxsplit=1)[1]

Here's a version of the script that doesn't use any lists:

import os


with open('C:\\Users\\config.txt') as f:
    for line in f:
        if '=' in line:
            path = line.split('=', maxsplit=1)[1].strip()
            components = path.split('\\')
            print(os.path.join(*components))
\$\endgroup\$
5
  • 2
    \$\begingroup\$ I suggest dropping the else: continue and finishing the if clause with print(os.path.join(*path.split('\\'))). \$\endgroup\$ Dec 15 '15 at 10:58
  • \$\begingroup\$ @200_success Yes, that's neater. Fixed. \$\endgroup\$
    – alexwlchan
    Dec 15 '15 at 11:05
  • \$\begingroup\$ I suggest f -> f.readlines() thay are equal but the second is more explicit \$\endgroup\$
    – Caridorc
    Dec 15 '15 at 14:50
  • 2
    \$\begingroup\$ @Caridorc Using f.readlines() loads the entire file into memory as a list before continuing; for line in f only reads a line at a time. It makes little difference for small files, but there's a big performance difference for large files. \$\endgroup\$
    – alexwlchan
    Dec 15 '15 at 14:53
  • \$\begingroup\$ @alexwlchan you're correct, I thought readlines gave an iterator, well thaks for the tip :) \$\endgroup\$
    – Caridorc
    Dec 15 '15 at 14:59

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.