trjhtr

I wrote this class to generate a random "image," similar to a QR code, using strings. How can I make it better (I am a beginner in programming)?

import random

""" N is the length of squared field to be generated"""
class genImg():
 def __init__(self, N):
 self.N = N
 self.field = 

 def genField(self):
 for i in range(self.N):
 row = 
 for j in range(self.N):
 row.append("|-|")
 self.field.append(row[:])

 def getField(self):
 return self.field

 def fillField(self):
 field = self.getField()
 result = ""
 for i in field:
 row = ""
 for j in i:
 randNum = random.randint(1,100)
 if randNum % 2 == 0 or randNum % 3 == 0:
 j = '|0|'
 row += str(j) + " "
 result += row[:-1] + "n"
 print(result)

You are doing unnecessary work by first looping over each row and each column to initialise the field to empty squares; and then looping over each row and each column to change the state based on a random condition. You can do it in one looping by assigning the "empty" or "filled" state based on said condition.

And while we're at it, you could simplify the condition as there is exactly 67 numbers that are either multiple of two or three between 1 and 100. So you could at least test if randNum <= 67 which would yield the same probability. But this probability is pretty close to $frac23$ so we can come up with an other approach: use a tupple composed of two " filled" patterns and an "empty" one and use random.choice instead.

You could also benefit a lot from the str.join method instead of concatenating the chosen pattern and a separator to a temporary string. It will help improve your memory usage. Just put the selected patterns in a list and use the join method of your separator to create the whole string at once. Better yet, join over a list-comprehension or a generator expression.

Lastly, you don't need a class to solve this problem as you don't have any state. The various attributes are only there to store temporary variables. A simple function will do:

import random


def generate_image(size, *, filled='|0|', empty='|-|'):
 patterns = filled, filled, empty

 return 'n'.join(
 ' '.join(random.choice(patterns) for _ in range(size))
 for _ in range(size)
 )


if __name__ == '__main__':
 print(generate_image(20))

1. genField() generates an empty field. My first assumption given it's name is that it generates a random field. Call it something like genEmptyField() to make this clearer.




2. fillField() does not work unless genField() is called first. Call
   genField() at the begining of fillField(). This will also ensure
   that subsequent calls to fillField() are still random.




3. fillField() does the work of both generating the code and printing
   it. Seperate these into two seperate functions.




4. Make a method __str__(self) of genImg() that returns the string representation of the image, so that the codes that you generate can be printed, rather than them doing the printing.




5. Consider calling fillField() in __init__() so that the user always has a useful image.




6. If you want a uniform random image, randNum % 2 == 0 or randNum % 3 == 0 can be replaced by random.getrandbits(1)




7. In order to seperate generation and representation, consider storing the array of points as bools and then convert them to the string of your choice in __str__(). This makes changing your output format much simpler if you want to do that later.




8. self.field.append(row[:]) is slower and harder to understand than self.field.extend(row[:])




9. The name genImg implies that this class generates images. In reality this class is an image and provides methods for generation. Consider naming it something like Image. It is generally a good idea to name your classes with nouns rather than verbs (i.e. ImgGenerator rather than genImg)