Reading a file line by line - Two tokens per entry

Question

Problem

Read a file and capture two tokens for every multi-line entry.

First token is hexcode with which the entry starts.

Second token is the remaining line(multi-line) of the entry. Trailing backslash indicates the continuation of line.

Typical entries of a file is as shown below:

0x0512ff01 R 5330 R XY.Zion, \
        "parseexp( {f 7},{S \"OWAUTO\"})", "0x0222ff02 -:-", \
        "parseexp( {g 7},{S \"TWFAB\"})", "0x0222ff02 -:-",  \
        "setfault", "0x0222ff03 -:-"
# whatever
0x0436ff02 I 5330  R XY.Yion, " \
                    ! ( strcat( {H 19.19.121.180}, {attr 0x11137f })  \
                            "default", "0x0222ff03 -:-"
0x0236ff03 G 50
0x0588ff03 H 49
0x0777ff03 R 34

There are such ~100 files to read

---

Solution

Input to this function is a generator that provides path, one at a time

def get_tokens(file_paths):

    for path in file_paths:
        ruleCapture = False
        with open(path) as f:

            for line in f:

                line=line.strip() # remove \n and other spaces
                if line.startswith('#'): # comments
                    continue
                elif (not line.endswith('\\')) and (ruleCapture == False):
                    event_rule_match =  re.search(r'(?<!\\\n)^(0[xX][0-9a-fA-F]+)([^\n]*)',line)
                    if event_rule_match:
                        yield event_rule_match.group(1), event_rule_match.group(2)
                elif line.endswith('\\') and ruleCapture == False:
                    buffer = line[0:-2]
                    ruleCapture = True
                elif line.endswith('\\') and ruleCapture == True:
                    buffer = ' '.join([buffer, line[0:-2]])
                elif (not line.endswith('\\')) and ruleCapture == True:
                    buffer = ' '.join([buffer, line])
                    ruleCapture = False
                    event_rule_match =  re.search(r'(?<!\\\n)^(0[xX][0-9a-fA-F]+)([^\n]*)',buffer)
                    yield event_rule_match.group(1), event_rule_match.group(2)
                    buffer=''

---

1) Can this code avoid regex?

2) Can elif structure still be optimized?

3) Can this code avoid reading the file line by line? Instead use f.read() and compile regex once....

Answer 1

1) Control flow clarity

if a and b: ...
elif a and not b: ...
elif not a and b: ...
elif not a and not b: ...

can be improved to

if a:
    if b: ...
    else: ...
else:
    if b: ...
    else: ...

2) Remove unnecessary function calls

' '.join([a, b]) can be simplified to a + ' ' + b. In your case, since you're appending b, you can do a += ' ' + b.

3) Implicit slicing notation

line[0:-2] can be line[:-2].

4) Consolidate constants

The regex string is a constant and is used multiple times. It can be moved to the top as a constant (or even outside the function).

5) Remove unnecessary comments

Assume the reader is familiar with built-in functions like str.split(). Thus, # remove \n and other spaces is unnecessary.

6) Readability

• Include exactly one space before and after an assignment. e.g line = line.strip().
• No need for if a == True or if a == False. Instead, do if a or if not a.
• Remove unnecessary line breaks near the top.
• Add spaces after commas in a function's parameters. e.g: f(a, b) instead of f(a,b).
• Pick either camelCase or under_scores for variable names. Don't use both.

Final result:

def get_tokens(file_paths):
    REGEX = r'(?<!\\\n)^(0[xX][0-9a-fA-F]+)([^\n]*)'

    for path in file_paths:
        rule_capture = False
        with open(path) as f:
            for line in f:
                line = line.strip()
                if line.startswith('#'): # ignore comments
                    continue

                if line.endswith('\\'):
                    if rule_capture:
                        buffer += ' ' + line[:-2]
                    else:
                        buffer = line[:-2]
                        rule_capture = True
                else:
                    if rule_capture:
                        buffer += ' ' + line
                        rule_capture = False
                        event_rule_match = re.search(REGEX, buffer)
                        yield event_rule_match.group(1), event_rule_match.group(2)
                        buffer = ''
                    else:
                        event_rule_match = re.search(REGEX, line)
                        if event_rule_match:
                            yield event_rule_match.group(1), event_rule_match.group(2)

Notice I didn't change any of the logic, but just moved things around or removed things. See if you can simplify the logic (e.g. removing unnecessary assignments, etc).

Answer 2

In addition to Logikable's answer, I'd also suggest that instead of having only the pattern string as a constant; compile and save the expression object as constant. This improves performance. From the docs on re.compile:

Compile a regular expression pattern into a regular expression object, which can be used for matching using its match() and search() methods, described below.

[...]

The sequence

prog = re.compile(pattern)
result = prog.match(string)

is equivalent to

result = re.match(pattern, string)

but using re.compile() and saving the resulting regular expression object for reuse is more efficient when the expression will be used several times in a single program.

Although, you only have a single pattern, so this is also valid:

The compiled versions of the most recent patterns passed to re.match(), re.search() or re.compile() are cached [...]

Answer 3

Answering your question by number

1) From your example file one could think that line.startswith('0') is sufficient. You could also try to convert the hex number int(line.split()[0],16) and catch for ValueError. But unless there is reason to avoid regex you can stick to it.

2) Logikables answer shows how to correctly use nested if-else instead of combinations of nested conditions in an elif rake. However even if you work line by line you should separate the tasks/stages you perform. The stages are reading, cleaning/decoding and interpreting/processing. your control flow should look like

for line in f:

    # cleaning and decoding file format
    if is_comment(line):
        continue
    store(buffer, line)

    # interpret/process
    if is_complete(buffer):
        interpret(buffer)
        clear(buffer)

thus resulting in something like

buffer = []
for line in f:

    line = line.strip()

    if line.startswith('#'): # ignore comments
        continue

    if line.endswith('\\'):
        buffer.append(line[:-2])
    else:
        buffer.append(line)

    if not line.endswith('\\'):
        event_rule_match = re.match(REGEX, ' '.join(buffer))
        if event_rule_match:
            yield event_rule_match.group(1), event_rule_match.group(2)
        else:
            # error handling or logging?
            pass
        buffer = []

this saves tons of error prone code duplication. Do not work on buffer in some cases and on line in others. always work on buffer. do not define re patterns multiple times. all this slight modifications and special cases have to be tested. avoid all thes flags and conditional buffer cleanups.

Code with duplication is unmaintainable and untestable. Do not repeat yourself, neither code nor regex patterns, nor numeric constants. If you cannot avoid to reuse code in other control blocks define functions and patterns outside and use those.

3a) Yes, you can avoid reading line by line. But don't dare to do so, seriously. What would be better on reading the whole file? nothing. What would be worse? You lose the ability to process big files. Get used to thinking of processing endless streams, this is superior in any case.