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.
1 Answer 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.
- 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.
- 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.
-
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– pockspocky2022年03月07日 13:39:13 +00:00Commented 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– Michael2022年03月07日 13:45:55 +00:00Commented 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\$200_success– 200_success2022年03月07日 19:51:22 +00:00Commented 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– pockspocky2022年03月08日 00:24:12 +00:00Commented Mar 8, 2022 at 0:24