With this algorithm I update an automatically generated verilog-file. The update within this file is done by commenting the assigned wires to specific ports of module instances.

Is there a better, more elegant or more optimized way to do this than with this algorithm?

file_name = "test.v"    # name of the verilog file
test = ".test"          # port name 1
tezt = ".tezt"          # port name 2
dummy = []              # buffer for the updated string

with open(file_name, "r+") as f:
    lines = f.readlines()
    f.truncate()                                 # clear the file
    for line in lines:
        if test in line or tezt in line:         # check if one of the ports is in the line
            if line[line.index('(')+1] != '/':   # check if the assigned wire is already is commented
                 for c in line:                  # update the line and comment the wire name within the brackets
                    if c == ')':
                    if c == '(':
                 line = line.replace(line, "".join(dummy))    # replace the old line with the new string
  • 1
    \$\begingroup\$ Welcome to Code Review! "... done by commenting the assigned wires ..." should likely be " ... connecting ...", no? \$\endgroup\$
    – AlexV
    Jun 14, 2019 at 13:15
  • \$\begingroup\$ @AlexV Looking at the dummy.append("*/"), it looks like the are commenting the wires. \$\endgroup\$
    – Peilonrayz
    Jun 14, 2019 at 13:26
  • \$\begingroup\$ @Peilonrayz On the second look that might actually be true :-) \$\endgroup\$
    – AlexV
    Jun 14, 2019 at 13:30

1 Answer 1

  • You don't need to manually call f.close(), that's what the with is for.
  • It looks like line = line.replace(line, "".join(dummy)) can just be line = "".join(dummy).
  • It's clearer to define dummy in the if statement:

    • This means that it's in the correct scope, meaning we don't have to look out for it being used elsewhere.
    • It also means that you can remove dummy.clear().
  • Personally I would merge your two if statements together, to reduce the arrow anti-pattern.
  • It looks like dummy and your for loop can be replaced with str.maketrans and str.translate.


file_name = "test.v"
test = ".test"
tezt = ".tezt"
trans_table = str.maketrans({'(': '(/*', ')': '*/)'})

with open(file_name, "r+") as f:
    lines = f.readlines()
    for line in lines:
        if ((test in line or tezt in line)
            and line[line.index('(') + 1] != '/'
            line = line.translate(trans_table)
  • \$\begingroup\$ Thank you for your improvements. I knew that it could be optimized. The two string methods are new to me, but it's good to know that they exist. \$\endgroup\$
    – Olupo
    Jun 17, 2019 at 5:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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