Skip to main content
fix tpos
Source Link
Snowbody
  • 8.5k
  • 23
  • 50

Okay I know I'm late to the party but I wanted to give my impressions on this code too.

  • Functions/methods are too long and are doing too much. When a function is longer than a screenful, it becomes very hard to keep everything in mind. Functions should obey the single responsibility principle: have one clearly defined task that it performs.
  • Too many magic numbers. There are numbers and strings scattered throughout the code and I have no idea where any of them come from or what they mean. Some of them have comments near them, but it's still inscrutable.
  • Lots of repeated code. For instance all the self.data.read('unitle'uintle:64') Maybe should be their own method.
  • Split out the generation of the string in show_info() into a new method __str__() since representing making the object readable. Then show_info() should just call str(self)` should just call str(self)
  • Need some error/exception detection in places where you work with files.
  • Don't call print() in the main logic of your code. What if you code has to be run some place where there isn't a terminal, such as a batch process or GUI? Instead look into using the logging module.

Okay I know I'm late to the party but I wanted to give my impressions on this code too.

  • Functions/methods are too long and are doing too much. When a function is longer than a screenful, it becomes very hard to keep everything in mind. Functions should obey the single responsibility principle: have one clearly defined task that it performs.
  • Too many magic numbers. There are numbers and strings scattered throughout the code and I have no idea where any of them come from or what they mean. Some of them have comments near them, but it's still inscrutable.
  • Lots of repeated code. For instance all the self.data.read('unitle:64') Maybe should be their own method.
  • Split out the generation of the string in show_info() into a new method __str__() since representing making the object readable. Then show_info() should just call str(self)`
  • Need some error/exception detection in places where you work with files.
  • Don't call print() in the main logic of your code. What if you code has to be run some place where there isn't a terminal, such as a batch process or GUI? Instead look into using the logging module.

Okay I know I'm late to the party but I wanted to give my impressions on this code too.

  • Functions/methods are too long and are doing too much. When a function is longer than a screenful, it becomes very hard to keep everything in mind. Functions should obey the single responsibility principle: have one clearly defined task that it performs.
  • Too many magic numbers. There are numbers and strings scattered throughout the code and I have no idea where any of them come from or what they mean. Some of them have comments near them, but it's still inscrutable.
  • Lots of repeated code. For instance all the self.data.read('uintle:64') Maybe should be their own method.
  • Split out the generation of the string in show_info() into a new method __str__() since representing making the object readable. Then show_info() should just call str(self)
  • Need some error/exception detection in places where you work with files.
  • Don't call print() in the main logic of your code. What if you code has to be run some place where there isn't a terminal, such as a batch process or GUI? Instead look into using the logging module.
Source Link
Snowbody
  • 8.5k
  • 23
  • 50

Okay I know I'm late to the party but I wanted to give my impressions on this code too.

  • Functions/methods are too long and are doing too much. When a function is longer than a screenful, it becomes very hard to keep everything in mind. Functions should obey the single responsibility principle: have one clearly defined task that it performs.
  • Too many magic numbers. There are numbers and strings scattered throughout the code and I have no idea where any of them come from or what they mean. Some of them have comments near them, but it's still inscrutable.
  • Lots of repeated code. For instance all the self.data.read('unitle:64') Maybe should be their own method.
  • Split out the generation of the string in show_info() into a new method __str__() since representing making the object readable. Then show_info() should just call str(self)`
  • Need some error/exception detection in places where you work with files.
  • Don't call print() in the main logic of your code. What if you code has to be run some place where there isn't a terminal, such as a batch process or GUI? Instead look into using the logging module.