Convert custom format to XML Template

Question

I have just finished writing a simple Python3 program which converts a custom input file (or multiple files within the same folder) to an XML template.

It works as it is, but I think that I somehow messed up the code as I had to open several files, append to them, close them, loop through their lines and so on.

I thought of splitting my code into methods, but I wasn't able to come up with a nicer solution. I'm sure there are better ways of doing what I just did, but I couldn't come up with any.

Here is the code:

import datetime
import os
import random
import string
import sys
from time import strftime

'''
The program is:
- verifying if a path exists (user input)
- if the path exists, loop through each files from that folder, else exit
- there are two types of files, and the only difference between them is a line which starts with ":23B:FCY"
- because there are two type of files, there will be two XML templates.
'''

# ask ouser for path
input_file = input('Please insert the path of the folder that holds all the files (e.g: D:/files_to_be_converted): ')

# some constants
LEN_OF_RANDOM_MSG_ID = 26
FIRST_PART_MSG_ID = 'RO000IO'

# playing with strings
random_msg_id = FIRST_PART_MSG_ID + ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(LEN_OF_RANDOM_MSG_ID))
random_pmt_inf_id = FIRST_PART_MSG_ID + ''.join(random.choice(string.digits + string.ascii_uppercase) for _ in range(LEN_OF_RANDOM_MSG_ID))

if not os.path.isdir(input_file):
    print("Inserted file path (" + input_file + ") does not exist !")
    print("Will exit now...")
    sys.exit()

# Do we even need two different templates?
template_one = """<tag1>%(msg_id)s</tag1>
<tag2>%(cre_dt_tm)s</tag2>
<tag3>%(nb_of_txs)s</tag3>
<tag4>%(ctrl_sum)s</tag4>
<tag5>%(nm)s</tag5>
<tag6>%(pmt_inf_id)s</tag6>
<tag7>%(reqd_exctn_dt)s</tag7>
"""

template_two = """<tag1>%(msg_id)s</tag1>
<tag2>%(cre_dt_tm)s</tag2>
<tag3>%(nb_of_txs)s</tag3>
<tag4>%(ctrl_sum)s</tag4>
<tag5>%(nm)s</tag5>
<tag6>%(pmt_inf_id)s</tag6>
<tag7>%(reqd_exctn_dt)s</tag7>
"""

for each_mt_file in os.listdir(input_file):
    with open(os.path.join(input_file, each_mt_file)) as mt_file:
        for line in mt_file:
            line = line.rstrip()
            if ":07:" in line:
                output_file = line[4:12]  # this will be part of each generated xml file

    if ":23B:FCY" in open(os.path.join(input_file, each_mt_file)).read():  # if there is a line which contains :23BFCY
        the_output_file = open("template1_" + output_file + ".xml", "a")  # name of the XML file
        template = template_one

    else:
        the_output_file = open("template2_" + output_file + ".xml", "a")
        template = template_two

    with open(os.path.join(input_file, each_mt_file)) as my_file:  # open each_file and start processing the lines
        for line in my_file:  # going through each line of each file
            if ":32A:" in line:  # if this field exists process it
                final_date = datetime.datetime.strptime(line[5:11], '%y%m%d').strftime('%Y-%m-%d')
                ctrlSum = line[14:]
                break  # break here, because there are multiple lines that start with ":32A:" and I want only the first
            if ":03:" in line:
                NbOfTxs = line[4:]
            if ":05:" in line:
                Nm = line[4:]

    CreDtTm = str(final_date) + "T" + str(strftime("%H:%M:%S"))
    ReqdExctnDt = str(final_date)
    the_output_file.write(template % {
        'msg_id': random_msg_id,
        'cre_dt_tm': CreDtTm,
        'nb_of_txs': NbOfTxs,
        'ctrl_sum': ctrlSum,
        'nm': Nm,
        'pmt_inf_id': random_pmt_inf_id,
        'reqd_exctn_dt': ReqdExctnDt
    })

And here is the custom format of the file:

:03:value1
:05:value2
junk
junk
:51:junk
junk
:07:OB25E294.jjj
:32A:160225val10.00
:99:junk
junkjunk
junk

and the second one, which is slightly different:

:03:value6
:05:value7
:*23B:FCY*
junk
junk
:51:junk
junk
:07:OB25E294.jjj
:32A:160225val10.00
:99:junk
junkjunk
junk

I'm asking for some code improvements / design patterns and mistakes that I surely did. I just feel like this code can be also split up in several files like template_1.py and template_2.py and then to import those in my main file but I'm not sure this is good practice.

Answer 1

Don't read from the same file three times when you can do it once!

with open(os.path.join(input_file, each_mt_file)) as mt_file:

if ":23B:FCY" in open(os.path.join(input_file, each_mt_file)).read():

with open(os.path.join(input_file, each_mt_file)) as my_file:

What makes this even worse is you only need one and you don't close the file!

And you don't close your the_output_file, seriously use with.

---

If you merge your two with loops into one you will be able to see that you should probably not do what you are.

It would also read as:

• Get last :07: and only take 8 characters after it.
• Get the first :32A:, use the first 6 characters for the date, the last for something else.
• Get the last :03:.
• Get the last :05:.

Which would be super simple and easy if you had a dictionary of lists. For the first one:

output_file = file_data["07"][-1][:8]

To be able to do this is simple. Use a defultdict and a regex(re).

You'd use a regex such as ^:([^:]+):(.*) to get the key and value. On some lines such as 'junkjunk' the regex will match nothing so we will want to guard against that. Due to the ^ it wont match a:23:b. If you don't know regex too well, the regex will look for a line beginning with : followed by anything, until it reaches another :. After the second : it will capture everything and set it as the value. And so :07:OB25E294.jjj would be ('07', 'OB25E294.jjj') in the group variable.
Finally it will add the data to a list so you can select the correct value to use. For example passing:

:1:value1
junk
junk
:1:value3
:2:junk
:1:value2

would be the same as setting:

file_data = {
    '1': [
        'value1',
        'value3',
        'value2'
    ],
    '2': ['junk']
}

Which can be implemented in as:

file_data = defaultdict(list)
for line in f:
    line_data = re.match('^:([^:]+):(.*)', line)
    if line_data is not None:
        groups = line_data.groups()
        file_data[groups[0]].append(groups[1])

You can then use what I said above. But there is one I didn't mention, :23B:. You want to know if any of them start with 'FCY':

any(i[:3] == "FCY" for i in file_data["23B"])

But you should get the general idea for the rest of them.

---

Finally template_one is the same as template_two. Seriously just define one. And for the love of format, use str.format!

You could do any of the following:

"<tag1>{msg_id}</tag1>".format(msg_id="my_id")
"<tag1>{0}</tag1>".format("my_id")
"<tag1>{}</tag1>".format("my_id")

I'd use the second one, to use:

XML_FORMAT = ("<tag1>{0}</tag1>\n"
              "<tag2>{1}</tag2>\n"
              "<tag3>{2}</tag3>\n"
              "<tag4>{3}</tag4>\n"
              "<tag5>{4}</tag5>\n"
              "<tag6>{5}</tag6>\n"
              "<tag7>{6}</tag7>\n")

And usage would be what you are doing without the awkward dictionary. Such as:

the_output_file.write(XML_FORMAT.format(
    random_msg_id,
    "{}T{}".format(final_date, strftime("%H:%M:%S")),
    file_data["03"][-1],
    line_32a[9:],
    file_data["05"][-1],
    random_pmt_inf_id,
    final_date
))

---

If you go onto split the function out into multiple functions I would make at least two and will demonstrate three.

The first would be wrapping your code in a function. I would also make reading your custom file a function so that you can re-use it. Finally I changed it to make all changes to the data in a separate function to the file output, this is as changes to the data can be confused with saving the data. But all in all it's mostly to have nicer easier to read chunks of code. Finally you may want to make a save function, and a file to file name function, but IMO that would decrease readability.

It's mostly down to personal preference here, but if you get duplicate code you should probably make a new function. Which we don't have here.

RANDOM_MSG_ID = random_msg_id
RANDOM_PMT_INF_ID = random_pmt_inf_id

def read_custom_file(path):
    file_data = defaultdict(list)
    with open(path) as f:
        for line in f:
            line_data = re.match('^:([^:]+):(.*)', line)
            if line_data is not None:
                groups = line_data.groups()
                file_data[groups[0]].append(groups[1])
    return file_data

def _extract_data(data):
    line_32a = data["32A"][0]
    final_date = datetime.datetime.strptime(line_32a[:6], '%y%m%d').strftime('%Y-%m-%d')
    return (
        RANDOM_MSG_ID,
        "{}T{}".format(final_date, strftime("%H:%M:%S")),
        data["03"][-1],
        line_32a[9:],
        data["05"][-1],
        RANDOM_PMT_INF_ID,
        final_date
    )

def custom_file_to_xml(directory):
    if not os.path.isdir(directory):
        return False
    
    for file_name in os.listdir(directory):
        file_data = read_custom_file(os.path.join(directory, file_name))
        data = _extract_data(file_data)
        
        # Save data
        output_file = file_data["07"][-1][:8]
        fcy = any(i[:3] == "FCY" for i in file_data["23B"])
        output_name = "template{}_{}.xml".format("1" if fcy else "2", output_file)
        with open(output_name, "a") as f:
            f.write(XML_FORMAT.format(*data))
    return True

if not custom_file_to_xml(input_file):
    print("Inserted file path (" + input_file + ") does not exist !")
    print("Will exit now...")