HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Printer Color Templates

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
templatescolorprinter

Problem

I'm new to Python and I just wrote my first program with OOP. The program works just fine and gives me what I want. Is the code clear? Can you review the style or anything else?

```
import numpy as np
import time, os, sys
import datetime as dt

class TemplateGenerator(object):
"""This is a general class to generate all of the templates"""

def excelWrtGE(self,*typos):
for typo in typos:
self.excelWriteGE.write(typo)
return

def create_KeywordsGE(self,list):
list1=list.split("/")
for x in list1:
list1=[list1.strip()for list1 in list1]
print list1
keyword1="Patrone, Toner aufladen "+str(list1[0])
keyword2="Patronen, CMYK, Set of, Ein"
keyword3="Set, of, Schwarz, Cyan, Magenta, Gelb, Photoblack"
keyword4="Remanufactured ink, Druckerpatronen Ersatz "
keyword5=str(list1[0])
print keyword5
i=1
while len(keyword5)=len(list1): break
if len(list1)==1: break
keyword5=keyword5+", "+str(list1[i])
if len(keyword5)>50:
keyword5=keyword5_old
break
i=i+1
keywords=keyword1+"\t"+keyword2+"\t"+keyword3+"\t"+keyword4+"\t"+keyword5 + "\t"
return keywords

def keyFeaturesGE(self,feat,inkNumber):
list1=feat.split(",")
for x in list1:
list1=[list1.strip()for list1 in list1]
if inkNumber==5:
feat1="Lieferumfang: : 1 Schwarz, 1 Cyan, 1 Magenta, 1 Gelb , 1 Photoblack"
if inkNumber==4:
feat1="Lieferumfang: : 1 Schwarz, 1 Cyan, 1 Magenta, 1 Gelb "
if inkNumber==1:
feat1="Lieferumfang: : 1 Cyan "
if inkNumber==2:
feat1="Lieferumfang: : 1 Magenta "
if inkNumber==3:
feat1="Lieferumfang: : 1 Gelb "
if inkNumber==0:
feat1="Lieferumfang: : 1 Schwarz "
if inkNumber==242:
feat1="Lieferumfang: : 4 Schwarz, 2 Cyan, 2 Magenta, 2 Gelb "
if inkNumber==54:
feat1="Lieferumfang: : 5 Schwarz, 5 Cyan, 5 Magenta, 5 Gelb "
if inkNumber==11:
feat1="Lieferumfang:

Solution

I could make out some parts that could be improved.

  • Good that you have used docstring for the class. You could do the same for the methods. In general, the code could have more comments where things are not obvious. Right now there are hardly any comments.



  • Python programmers prefer functions_named_like_this() rather than functionsNamedLikeThis().



  • __init__() should preferably be at the start of the class. Those who read the code will want to know how it is initialized before looking at other things.



  • Is this really necessary: super(TemplateGenerator, self).__init__() ? I don't know the answer but I believe the base class __init__ method either does nothing useful or is invoked by default.



  • A better way to open/close files is using the with statement. That way, you never need to worry about files you have forgotten to close.



  • header="Titles\t"+"Price equal to title:\t"+"Key Features1\t"+"Key Features2\t"+"Key Features3\t"+"Key Features4\t"+"Key Feature5\t"+"keyword1\t"+"keyword2\t"+"keyword3\t"+"keyword4\t"+"keyword5\t"+"Manufacturer Part Number\t"+"Description\t"+"Picture Link\n": this is definitely not Pythonic. Use instead a list and then call '\t'.join() on that list.



  • (x+2.5)1.261.21.41.2: since this is used in many places, put it into a method.



  • ElementsOfPrinter[]: this is really cryptic since it's hard to remember the semantics of each position. It would be better to extract them into named variables. Example: x,y,z = position, assuming position is a list of 3 items.



  • list1=[list1.strip()for list1 in list1]: nothing wrong here but it is better for readability to use different variable names.



  • test0, test1, ...: can be converted to a list instead. Also, Python allows you to have multiline strings. No need to type them out in long lines.



  • if inkNumber==4: : convert to elif.

Context

StackExchange Code Review Q#70989, answer score: 2

Revisions (0)

No revisions yet.