5
\$\begingroup\$

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):
 pass
 def possiblePathsGeneration(self, numOfPoints = 5):
 flag = False
 alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
 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
 break
 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
 else:
 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
 break
 if accumulator >= terminateNum: # Determines when to leave the loop
 break
 points.append(y * x) # Outputs point
 accumulator += 1
 if flag != True & accumulator != terminateNum | accumulator <= terminateNum: # Determines if we have more whole alphabets
 continue
 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
 break
 points.append(y * x)
 else:
 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
 temporaryPoints.pop(0)
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.

200_success
145k22 gold badges190 silver badges478 bronze badges
asked Mar 7, 2022 at 13:02
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$
  1. Strings

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

import string
alphabet = string.ascii_uppercase
print(alphabet)

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):
 path.append(combo)

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.

answered Mar 7, 2022 at 13:23
\$\endgroup\$
4
  • 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\$ Commented 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\$ Commented 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\$ Commented Mar 7, 2022 at 19:51
  • \$\begingroup\$ @200_success thanks, I will play with it and see if it produces what I want \$\endgroup\$ Commented Mar 8, 2022 at 0:24

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.