Please review my code: imged (Image Editor)

This is the place to post any code that you want to share with the community. Only completed scripts should be posted here.

Please review my code: imged (Image Editor)

Postby africanHseSnk » Tue Jul 02, 2013 5:55 pm

Hello,

I wrote this code for a Python job interview (last minute).
I was told I would have the code reviewed. It didn't happen!
Please could you review this and get back to me on what I could have done better etc.
Any help would be most appreciated.

Code: Select all

import time
import itertools #used for ID generation
import logging

#TODO:
#1) show needs to cater for new cell object
#2) new region array instead of imgarray (region array = dictionary type thing, region(object, with colour value), then list all cells by reference), new objection, has new region with colour, look for all regions with colour X change to Y, should then change ref value in cell (hack to region), then re-render (i.e. new show method)

#Changed the way cell is; now, 3dim array. Layers needed. Region will have colour still and each object created will be part of a new region.
#created diagram to sort it out what to do
#removed new layer (3d array). Have id for region now.

rowLimit=250

#colour set
colourSET = {"A","B","C","D","E","F","G","H","I","J","K","L","M","N","O","P","Q","S","T","U","V","W","X","Y","Z"} #O means white
defaultCOLOUR = "O" #O is white and default

class region(object):
   _colour_state = defaultCOLOUR
   _genID = itertools.count(1) #first gen id = 1
   _id = 0
   #initiate
   def __init__(self,rid=-1):
      self._colour_state = defaultCOLOUR
      if str(rid) == "-1":
         self._id = next(self._genID)
      else:
         self._id = rid
   def setColour(self,col):
      if col.isalnum():
         if col in colourSET:
            self._colour_state = col
         else:
            print "imged internal error, set region colour: illegal colour value ("+str(col)+")."
   def setRegion(self,rid=-1):
      #print "rid="+str(rid)
      if str(rid) == "-1":
         print "imged internal error, set region id: illegal id value."
      else:
         self._id = rid
   def printColour(self):
      print self._colour_state
   def printRegion(self):
      print self._id
   def getRegion(self):
      return self._id
   def getColour(self):
      return self._colour_state


class cell(object):
   _numNeighbours = 1
   _neighbourValueArr = []
   _cellRegion = region(0)
   #initiate
   def __init__(self,numNeighs=8,val=defaultCOLOUR,rid=0):
      #has eight neighbours (N, NE, E, SE, S, SW, W, NW), ref 0-1-2-3-4-5-6-7
      for i in range(0,numNeighs):
         self._neighbourValueArr.append(val)
      self._value=val
      self._numNeighbours=numNeighs
      self._cellRegion.setRegion(rid)
   def setValue(self,val="",rid=-1):
      if val.isalnum():
         if rid == self._cellRegion.getRegion():
            #change whole of region's colour
            self._cellRegion.setColour(val)
         else:
            #new region needed with possible new colour (new cell/line/blob)
            self._cellRegion = region(rid)
            self._cellRegion.setColour(val)
      else:
         print "imged internal error, set cell value: illegal cell value."
   def setChangeRegion(self,rid=-1):
      self._cellRegion.setRegion(rid)
   def setNeighbours(self,listNeighs):
      issue = 0
      if len(listNeighs) != self._numNeighbours:
         issue = 1
      for neighs in listNeighs:
         if (str(neighs).isalnum() != true) and ( len(str(neighs)) != 1):
            issue = 1
      if issue == 0:
         self._neighbourValueArr[:] = listNeighs
      else:
         print "Internal issue, neighbours asssign, list given has issue."
   def seeCellValue(self):
      print "value="+str(self._cellRegion.getColour)
   def seeNeighbours(self):
      for neighs in self._neighbourValueArr:
         print "neighs="+str(neighs)
   def getColour(self):
      return self._cellRegion.getColour()

class imageArr(object):
   _imgArray=[[]]
   _maxM = -1 #have as array refs (0-1-2-etc.)
   _maxN = -1 #have as array refs (0-1-2-etc.)
    #initate
   def __init__(self,M=5,N=5):
      #M column, N row (removed, L layer)
      #self._imgArray = [[[cell() for i in xrange(L)] for i in xrange(M)] for i in xrange(N)]
      self._imgArray = [[cell() for i in xrange(M)] for i in xrange(N)]
      #a 5 length ceates 0-1-2-3-4 index, rem ref first cell top left 1,1
      #e.g. imgArray[3][2], hits 0-1-2-3 row, and 0-1-2 column
      self._maxM = M - 1
      self._maxN = N - 1
   def seeImage(self):
      rtnStr = ""
      outStr = "=>"
      print outStr
      rtnStr = outStr
      for line in self._imgArray:
         outStr=""
         for cell in line:
            outStr=outStr+""+str(cell.getColour())
         print outStr
         rtnStr = rtnStr + "\n" + outStr
      return rtnStr
   def setCell(self,M=0,N=0,value="",rid=-1):
      M=M-1 #cater for 1-2-3-etc, with array ref of 0-1-2-etc
      N=N-1
      #L=L-1
      if ( M > -1 and N > -1 ) and ( M <= self._maxM and N <= self._maxN) :
         if value.isalnum():
            self._imgArray[N][M].setValue(value,rid)
         else:
            print "imged internal error, set cell: illegal cell value."
      else:
         print "imged internal error, set cell: illegal cell reference, or no values given."
   def fillOverrideColour(self,M=0,N=0,value="",rid=-1):
      #first, get cells current colour
      M=M-1 #cater for 1-2-3-etc, with array ref of 0-1-2-etc
      N=N-1
      if ( M > -1 and N > -1 ) and ( M <= self._maxM and N <= self._maxN) :
         changeCol = self._imgArray[N][M].getColour() #imgArray[3][2], hits 0-1-2-3 row, and 0-1-2 column
         #second, set cell to new colour
         self.setCell(M+1,N+1,value,rid)
         #for fill, need to loop through cells, if it's region is colour C need to change to that colour (i.e. make the same region)
         for line in self._imgArray:
            for cell in line:
               #print "is cell colour "+str(cell.getColour())+" same as "+changeCol+"?"
               if cell.getColour() == changeCol:
                  #print "yes, change to "+str(rid)+" with colour of "+value
                  cell.setValue(value,rid)
      else:
         print "imged internal error, fill cell: illegal cell reference, or no values given."
   def clear(self):
      for line in self._imgArray:
            for cell in line:
               cell.setValue(defaultCOLOUR)
   def getMax(self, axis=""):
      if axis == "M":
         return self._maxM
      elif axis == "N":
         return self._maxN
      else:
         print "imged internal error, unknown axis to get max."
   def drawLine(self,staticAx=-1,fromAx=-1,toAx=-1,axis="",value="",rid=-1):
      #print "check inputs staticAx="+str(staticAx)+" fromax="+str(fromAx)+" toAx="+str(toAx)+" axis="+str(axis)+" value="+str(value)+" rid="+str(rid)
      maxAxisM=self.getMax("M")+1
      maxAxisN=self.getMax("N")+1
      maxAx = -1
      maxStatic = -1
      checkOk=False
      if (axis == "M" or axis == "N") and (fromAx < toAx) and staticAx > -1:
         #check that values are ok before loop
         toAx = toAx + 1
         #print "IN LINE DRAW"
         if axis == "M":
            maxAx = maxAxisN
            maxStatic = maxAxisM
         elif axis == "N":
            maxAx = maxAxisM
            maxStatic = maxAxisN
         #print "check all maxes for axis "+str(axis)+" maxax="+str(maxAx)+" maxstatic="+str(maxStatic)
         if maxAx > -1 and maxStatic > -1:
            if (staticAx <= maxStatic) and fromAx > 0 and toAx > 0 and fromAx <= maxAx and toAx <= maxAx:
               for lpAxis in xrange(fromAx,toAx):
                  #print "loop actived "+str(lpAxis)
                  if axis == "M":
                     self.setCell(staticAx,lpAxis,value,rid)
                  elif axis == "N":
                     self.setCell(lpAxis,staticAx,value,rid)
            else:
               print "imged internal error, invalid values for line drawing (2)."
         else:
            print  "imged internal error, invalid values for line drawing (3)."
      else:
         print "imged internal error, invalid values for line drawing (1)."

#20130406 TJC: while loop UI, as per suggested off: http://wiki.python.org/moin/WhileLoop
print "Welcome to Image Editor (imged). Please enter commands after the > symbol."
#logfile
logging.basicConfig(filename='imged.log',level=logging.DEBUG)
logtimeformat = "%Y-%m-%d_%H:%M:%S"
#logging.info(str(datetime.datetime.now()) + " Start")
def logstuff(logtxt,level):
   if level.lower() == "i":
      logging.info(str(time.strftime(logtimeformat, time.localtime())+ ": "+logtxt))
   elif level.lower() == "e":
      logging.error(str(time.strftime(logtimeformat, time.localtime())+ ": "+logtxt))
   elif level.lower() == "w":
      logging.warn(str(time.strftime(logtimeformat, time.localtime())+ ": "+logtxt))
   elif level.lower() == "c":
      logging.critical(str(time.strftime(logtimeformat, time.localtime())+ ": "+logtxt))
   else:
      print "Unknown logging level."
      logging.error(str(time.strftime(logtimeformat, time.localtime())+ ": Unknown logging level (level=" +level+ ")" ) )

logstuff("Start","i")
issue = ""
img_exists=False

while 1:
   n = raw_input(">")
   if n <> "":
      if n.strip() == 'X':
         logstuff("Exit","i")
         #print 'exit'
         break
      elif n.strip()[0] == 'I':
         logstuff("Command: Create New (I), (input:'"+str(n)+"').","i")
         if len(n) < 4:
            issue = "Incorrect syntax for command I, lack of input, should be 'I M N' (input:'"+str(n)+"')."
            print issue
            logstuff("Command: Create New (I),"+issue,"e")
         else:
            if n.strip().count(" ") <> 2:
               issue = "Incorrect syntax for command, only 2 spaces required between columns (M) and rows (N), should be 'I M N' (input:'"+str(n)+"')."
               print issue
               logstuff("Command: Create New (I),"+issue,"e")
            else:
               inCreateCmd = n.strip().split(' ')
               inM=0
               inN=0
               if inCreateCmd[1].isdigit():
                  #isdigit does not accept floats and does not accept blanks
                  inM=int(inCreateCmd[1])
               if inCreateCmd[2].isdigit():
                  inN=int(inCreateCmd[2])
               if inM > 0 and inN > 0 and inN <= rowLimit:
                  img=imageArr(inM,inN)
                  img_exists=True
               elif inN > rowLimit:
                  issue = "Rows (N) must be less than or equal to "+ str(rowLimit) +" (input:'"+str(inN)+"')."
                  print issue
                  logstuff("Command: Create New (I),"+issue,"e")
               else:
                  issue = "Issue with input (input:'"+str(n)+"'), please try again."
                  print issue
                  logstuff("Command: Create New (I),"+issue,"e")
      elif n.strip()[0] == 'C' and img_exists:
         #print 'clear'
         logstuff("Command: Clear (C)","i")
         img.clear()
      elif n.strip()[0] == 'L' and img_exists:
         #print 'colour single pixel'
         logstuff("Command: Colour single pixel (L), (input:'"+str(n)+"').","i")
         #img.setCell(2,3,"A")
         if len(n) < 6:
            issue = "Incorrect syntax for command, lack of input, should be 'L M N C' (input:'"+str(n)+"')."
            print issue
            logstuff("Command: Colour single pixel (L),Incorrect syntax for command, lack of input, should be 'L M N C' (input:'"+str(n)+"').","e")
         else:
            if n.strip().count(" ") <> 3:
               issue = "Incorrect syntax for command L, only 3 spaces required between columns (M), rows (N) and colour (C), should be 'L M N C' (input:'"+str(n)+"')."
               print issue
               logstuff("Command: Colour single pixel (L), " + issue,"e")
            else:
               inColourCmd = n.strip().split(' ')
               inM=0
               inN=0
               inValue=""
               if inColourCmd[1].isdigit():
                  #isdigit does not accept floats and does not accetp blanks
                  inM=int(inColourCmd[1])
               if inColourCmd[2].isdigit():
                  inN=int(inColourCmd[2])
               if inColourCmd[3].isalnum() and len(inColourCmd[3]) == 1:
                  inValue=str(inColourCmd[3]).upper()
               if inM > 0 and inN > 0 and inValue <> "":
                  #create region
                  newregion = region()
                  newregion.setColour(inValue)
                  newrid = newregion.getRegion()
                  #print "new region of "+str(newrid)+" of value "+inValue+" at cell M "+str(inM)+" and N "+str(inN)
                  #set cell
                  img.setCell(inM,inN,inValue,newrid)
               else:
                  issue = "Issue with input (input:'"+str(n)+"'), please try again."
                  print issue
                  logstuff("Command: Colour single pixel (L), " + issue,"e")
      elif n.strip()[0] == 'V' and img_exists:
         #print 'draw coloured line vertical'
         logstuff("Command: Draw vertical coloured line (V), (input:'"+str(n)+"').","i")
         if n.strip().count(" ") <> 4:
            issue = "Incorrect syntax for command, only 4 spaces required between inputs, the column for the vertical line (M), for the row to start (Y1) to the row where the line ends (Y2), with colour (C), should be 'V M Y1 Y2 C' (input:'"+str(n)+"')."
            print issue
            logstuff("Command: Draw vertical coloured line (V), "+issue,"e")
         else:
            inVertCmd = n.strip().split(' ')
            maxM=img.getMax("M")+1
            maxN=img.getMax("N")+1
            colM=0
            begN=0
            endN=0
            lineCol=""
            issue=""
            if inVertCmd[1].isdigit():
               colM=int(inVertCmd[1])
               if colM > maxM:
                  if issue <> "":
                     issue=issue+"\n"
                  issue="The column to have the line, is greater than the Image size (max="+str(maxM)+",request="+str(colM)+")."
            if inVertCmd[2].isdigit():
               begN=int(inVertCmd[2])
               if begN > maxN:
                  if issue <> "":
                     issue=issue+"\n"
                  issue="The start of the line is greater than the Image size (max="+str(maxN)+",request="+str(begN)+")."
            if inVertCmd[3].isdigit():
               endN=int(inVertCmd[3])
               if endN > maxN:
                  if issue <> "":
                     issue=issue+"\n"
                  issue="The end of the line is greater than the Image size (max="+str(maxN)+",request="+str(endN)+")."
            if inVertCmd[4].isalnum():
               if inVertCmd[4].upper() in colourSET:
                  lineCol=inVertCmd[4].upper()
            if colM > 0 and begN > 0 and endN > 0 and lineCol <> "" and begN < endN and issue == "":
               #new region
               newregion = region()
               newregion.setColour(lineCol)
               newrid = newregion.getRegion()
               #draw line
               img.drawLine(colM,begN,endN,"M",lineCol,newrid)
            else:
               if issue == "":
                  issue = "Issue with input (input:'"+str(n)+"'), please try again."
               print issue
               logstuff("Command: Draw vertical coloured line (V), "+issue,"e")
      elif n.strip()[0] == 'H' and img_exists:
         #print 'draw coloured line horizontal'
         logstuff("Command: Draw horizontal coloured line (H), (input:'"+str(n)+"').","i")
         if n.strip().count(" ") <> 4:
            print "Incorrect syntax for command, only 4 spaces required between inputs, the row for the horizontal line (N), for the column to start (X1) to the column where the line ends (X2), with colour (C), should be 'H N X1 X2 C' (input:'"+str(n)+"')."
            print issue
            logstuff("Command: Draw horizontal coloured line (H), "+issue,"e")
         else:
            inHorizCmd = n.strip().split(' ')
            maxM=img.getMax("M")+1
            maxN=img.getMax("N")+1
            rowN=0
            begM=0
            endM=0
            lineCol=""
            issue=""
            if inHorizCmd[1].isdigit():
               begM=int(inHorizCmd[1])
               if begM > maxM:
                  issue="Start of the line is greater than the Image size (max="+str(maxM)+",request="+str(begM)+")."
            if inHorizCmd[2].isdigit():
               endM=int(inHorizCmd[2])
               if endM > maxM:
                  if issue <> "":
                     issue=issue+"\n"
                  issue=issue+"End of the line is greater than the Image size (max="+str(maxM)+",request="+str(endM)+")."
            if inHorizCmd[3].isdigit():
               rowN=int(inHorizCmd[3])
               if rowN > maxN:
                  if issue <> "":
                     issue=issue+"\n"
                  issue=issue+"Row to have the line, is greater than the Image size (max="+str(maxN)+",request="+str(rowN)+")."
            if inHorizCmd[4].isalnum():
               if inHorizCmd[4].upper() in colourSET:
                  lineCol=inHorizCmd[4].upper()
               else:
                  if issue <> "":
                     issue=issue+"\n"
                  issue=issue+"Value for colour is invalid (value="+str(lineCol)+")."
            if rowN > 0 and begM > 0 and endM > 0 and lineCol <> "" and begM < endM and issue == "":
               #new region
               newregion = region()
               newregion.setColour(lineCol)
               newrid = newregion.getRegion()
               #draw line
               img.drawLine(rowN,begM,endM,"N",lineCol,newrid)
            else:
               if issue == "":
                  issue = "Issue with input (input:'"+str(n)+"'), please try again."
               print issue
               logstuff("Command: Draw horizontal coloured line (H), "+issue,"e")
      elif n.strip()[0] == 'F' and img_exists:
         #print 'fill the region for the specified cell and any with the same colour, before the fill'
         logstuff("Command: Fill the region for the specified cell and any with the same colour (F), (input:'"+str(n)+"').","i")
         if n.strip().count(" ") <> 3:
            issue = "Incorrect syntax for command F, only 3 spaces required between columns (M), rows (N) and colour (C), should be 'F M N C' (input:'"+str(n)+"')."
            print issue
            logstuff("Command: Fill the region (F), "+issue,"e")
         else:
            inFillCmd = n.strip().split(' ')
            inM=0
            inN=0
            inValue=""
            if inFillCmd[1].isdigit():
               #isdigit does not accept floats and does not accetp blanks
               inM=int(inFillCmd[1])
            if inFillCmd[2].isdigit():
               inN=int(inFillCmd[2])
            if inFillCmd[3].isalnum() and len(inFillCmd[3]) == 1:
               inValue=str(inFillCmd[3]).upper()
            if inM > 0 and inN > 0 and inValue <> "":
               #create region
               newregion = region()
               newregion.setColour(inValue)
               newrid = newregion.getRegion()
               #print "fill colour: new region "+str(newrid)+" of value "+inValue+" at cell M "+str(inM)+" and N "+str(inN)
               #set cell
               img.fillOverrideColour(inM,inN,inValue,newrid)
            else:
               issue = "Issue with input (input:'"+str(n)+"'), please try again."
               print issue
               logstuff("Command: Fill the region (F), "+issue,"e")
      elif n.strip()[0] == 'S' and img_exists:
         #print 'show content'
         output = ""
         logstuff("Command: Show content (S)","i")
         output = img.seeImage()
         logstuff("[OUTPUT]\n"+output+"\n[/OUTPUT].","i")
      elif n.strip()[0].islower():
         issue = "Issue with input (input:'"+str(n)+"'); the command is lowercase! Please try again."
         print issue
         logstuff("Command: , "+issue,"e")
      elif not img_exists:
         issue = "No image exists, please create one."
         print issue
         logstuff(issue+" (input:'"+str(n)+"').","e")
      else:
         issue = 'Unknown command, please try again.'
         print issue
         logstuff(issue+" (input:'"+str(n)+"').","e")
   else:
      issue = 'No command inputted; commands are: I (Create image), C (Clear image), S (Show image), L (Colourise a cell of the image), V (Draw vertical line), H (Draw horizontal line), F (Fill a region), X (Exit this program). Please try again.'
      print issue
      logstuff(issue+" (input:'"+str(n)+"').","e")


User avatar
africanHseSnk
 
Posts: 2
Joined: Tue Jul 02, 2013 5:51 pm

Re: Please review my code: imged (Image Editor)

Postby Yoriz » Tue Jul 02, 2013 7:28 pm

Initial thoughts are pep8 and looking at your avatar take your head out your backside :lol:
New Users, Read This
Join the #python-forum IRC channel on irc.freenode.net!
Spam topic disapproval technician
Windows7, Python 2.7.4., WxPython 2.9.5.0., some Python 3.3
User avatar
Yoriz
 
Posts: 571
Joined: Fri Feb 08, 2013 1:35 am
Location: UK

Re: Please review my code: imged (Image Editor)

Postby africanHseSnk » Tue Jul 02, 2013 8:32 pm

Coding layout conventions: Thanks! Normally it would look ok. I'm not sure what happened to the code rendering in this. Anyway, in Notepad++ in Python viewing mode, I think it's slightly easier to read. I do agree to the 'readable' code convention, but I disagree with convention nazism (Godwin's law). I've spotted a couple of things I could do with rearrangement with the code to make it slightly more concise, but from my coding experience (10 years), I've seen a lot worse.
Conclusion: I agree that the code shown here is difficult to read (no fault of anyone's, but my own; should have reviewed before submit).
I wave the flag of 'I give up' and go on my merry way.

Thank you for your help, I appreciate any guidance, and this hit the spot.
User avatar
africanHseSnk
 
Posts: 2
Joined: Tue Jul 02, 2013 5:51 pm


Return to Completed Scripts

Who is online

Users browsing this forum: No registered users and 2 guests