I wrote this code in 4 days, I want to see if I can optimize this code some more, and have some suggestion on what can I improve.

This code takes in any number of points then naming them alphabetically, when the points exceeds 26 the letters will repeat themselves AA BB CC, AAA, BBB, CCC, etc., then through combinations getting all paths between these points.

['A B', 'A C', 'A D', 'A E', 'B C', 'B D', 'B E', 'C D', 'C E', 'D E'] 
# This is only 5 points

The code:

class Distance:
    def __init__(self):

    def possiblePathsGeneration(self, numOfPoints = 5):

        flag = False
        accumulator = 0
        terminateNum = 0
        paths = []
        points = []
        temporaryStorage = ""
        flag2 = 0
        self.numOfPoints = numOfPoints

        if self.numOfPoints > 26:                                   # To see if it exceeds the alphabet

            points = list(alphabet[:26])                            # filling in the list with existing alphabet
            for x in range(2, ((self.numOfPoints - 1) // 26) + 2):  # How many times the letter will repeat itself (You will see in the output what i mean)

                for y in alphabet[:26]:                             # repeats over the alphabet

                    if flag2 == 1:                                  # Prevents another whole alphabet generating

                    if self.numOfPoints % 26 > 0 and (self.numOfPoints - 26) // 26 < 1:     # To see if it has any spare letters that does not form a full alphabet
                        terminateNum = self.numOfPoints % 26        # calculates how many spare letters are left out and sets it as a termination number for later
                        flag = True     # Sets a flag which makes one of the if statments false which allows execution of later programs

                        terminateNum = (self.numOfPoints - 26) // 26        # Getting the times that the alphabet has to iterate through

                    if flag == True and self.numOfPoints % 26 > 0 & (self.numOfPoints - 26) // 26 < 1:      # To see if we have a whole alphabet

                    if accumulator >= terminateNum:                 # Determines when to leave the loop

                    points.append(y * x)                            # Outputs point
                accumulator += 1
                if flag != True & accumulator != terminateNum | accumulator <= terminateNum:      # Determines if we have more whole alphabets

                terminateNum = self.numOfPoints % 26        # Resets number of letters to generate
                for y in alphabet[:terminateNum]:                   # outputs the spares
                    flag2 += 1
                    if flag2 == 1 and not(self.numOfPoints < 52):       # prevents generation of extra letters
                    points.append(y * x)

            points = list(alphabet[:self.numOfPoints])
        temporaryPoints = [x for x in points]
        for x in points:
            for y in temporaryPoints[1:]:
                paths.append(x + " " + y)
                temporaryStorage = x + " " + y
                yield temporaryStorage

distance = Distance()
print([x for x in distance.possiblePathsGeneration()])

I have tested this code a few times, it doesn't have any bugs that I know of.

The reason I use classes is that this is a part of the actual code, and using classes is convenient for later when I want to do some calculations.


1 Answer 1

  1. Strings

instead of inserting strings, you should use the python library.

import string

alphabet = string.ascii_uppercase

this will reduce the likely hood of typos.

  1. Naming

One thing I would consider is your naming objects. For instance, you name the class Distance, yet I don't see anything related to a measurement between two points. This could possibly be only part of the code, and you will have a measurement function, but then the class isn't strictly a measurement tool and also generates paths. So perhaps it should have a different name either way.

I might be overthinking it, but it will help with readability when it comes time for others to pick up your code and understand what it's supposed to do.

  1. iteration

another python tool to look at is itertools. You can use this for better ability to iterate over lists, etc. in your case it has a combination function.

import itertools

path = []
points = ['a', 'b', 'c', 'd', 'e']

for combo in itertools.combinations(points, 2):

this also stores your end result as a list of lists. Each path is a list. I personally dislike storing anything as a string, unless absolutely necessary or if it's actually a word/sentence.

  • 1
    \$\begingroup\$ This was really useful, thank you. I haven't played with itertools yet so I am not familiar with it but still, thanks \$\endgroup\$
    – pockspocky
    Mar 7, 2022 at 13:39
  • 1
    \$\begingroup\$ thanks. I'll update my answer with more if I think of other things. make sure to upvote if you like it. \$\endgroup\$
    – Michael
    Mar 7, 2022 at 13:45
  • 2
    \$\begingroup\$ Even simpler, treat strings as a sequence of characters, and also construct the result all at once instead of appending. path = list(itertools.combinations('abcde', 2)) \$\endgroup\$ Mar 7, 2022 at 19:51
  • \$\begingroup\$ @200_success thanks, I will play with it and see if it produces what I want \$\endgroup\$
    – pockspocky
    Mar 8, 2022 at 0:24

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.