1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85
def montage(): topLeft = makePicture(getMediaPath("images-1.jpg")) # Top left picture, type in later topRight = makePicture(getMediaPath("images-2.jpg")) # Top Right picture, bottomLeft = makePicture(getMediaPath("images-3.jpg")) # Bottom Left picture bottomRight = makePicture(getMediaPath("images-4.jpg")) # Bottom right picture middlePicture = makePicture(getMediaPath("images.jpg")) # Middle picture width = getWidth(topLeft) height = getHeight(topLeft) canvas = makeEmptyPicture(width*3, height*3) left = 1 top = 1 midX = width +1 midY = height +1 def posterize(pic): # simplifies the color range for pixel in getPixels(pic): if getRed(pixel) > 127: redness = 255 else: redness = 0 if getGreen(pixel) > 127: greenness = 255 else: greenness = 0 if getBlue(pixel) > 127: blueness = 255 else: blueness = 0 setRed(pixel, redness) setGreen(pixel, greenness) setBlue(pixel, blueness) return pic def grayscale(gPic): #sets RGB values equal to one another for p in getPixels(gPic): intensity = (getRed(p) + getGreen(p) + getBlue(p))/3 setColor(p, makeColor(intensity, intensity, intensity)) return gPic def negative(nPic): # makes all the colors opposite original values for px in getPixels(nPic): red1 = getRed(px) green1 = getGreen(px) blue1 = getBlue(px) negColor = makeColor(255-red1, 255-green1, 255-blue1) setColor(px, negColor) return nPic def sunset(sPic): # decreases blue and green to add a warmer red tone for p in getPixels(sPic): value = getBlue(p) setBlue(p, value*0.07) value1 = getGreen(p) setGreen(p, value1*0.07) return sPic def copyPicture(littlePic, bg, x, y): for destX in range(getWidth(littlePic)): for destY in range(getHeight(littlePic)): fromPixel = getPixel(littlePic, destX+1, destY+1) destPixel = getPixel(bg, destX+x, destY+y) setColor(destPixel, getColor(fromPixel)) #posterized pic top left #posterizedTL = posterize(TopLeft) copyPicture(posterize(topLeft), canvas, left, top) #grayscale pic top right #grayscaleTR = Grayscale(topRight) copyPicture(grayscale(topRight), canvas, midX, top) #negative pic bottom left #negativeBL = negative(bottomLeft) copyPicture(negative(bottomLeft), canvas, left, midY) #sunset pic bottom right #sunsetBR = sunset(bottomRight) copyPicture(sunset(bottomRight), canvas, midX, midY) # "original' pic copyPicture(middlePicture, canvas, midX/2, midY/2) return canvas
Refactorings
No refactoring yet !
Berco Beute
September 29, 2007, September 29, 2007 02:01, permalink
Just a simple enhancement (also to test out this site :) )
1
top, left = 1, 1
Darius Damalakas
September 29, 2007, September 29, 2007 03:06, permalink
What do you want to refactor here?
If you have submitted the code here, then I make an assumption that there's a problem with that code, or it could be made more nice, shorter, or more generic.
What I see here is that the code is ok in itself. Except that the picture modifying methods should be moved into separate module. But aside from this nothing more can be told
Daniel K
September 29, 2007, September 29, 2007 11:07, permalink
If the module you are calling included a generic "getColor(colorName, pixels)" method that took getColor("red",p) and returned the same value as getRed(p) (as well as an analogous setColor method), a lot of your functions could be simpler. It would be worth writing one these functions if there were a lot more effects being done. Also, your code would be more readable without comments where the code is pretty well self-commenting (at bottom).
If this is for a school project, please don't hand it in as your own work.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57
def montage(): topLeft = makePicture(getMediaPath("images-1.jpg")) # Top left picture, type in later topRight = makePicture(getMediaPath("images-2.jpg")) # Top Right picture, bottomLeft = makePicture(getMediaPath("images-3.jpg")) # Bottom Left picture bottomRight = makePicture(getMediaPath("images-4.jpg")) # Bottom right picture middlePicture = makePicture(getMediaPath("images.jpg")) # Middle picture width = getWidth(topLeft) height = getHeight(topLeft) canvas = makeEmptyPicture(width*3, height*3) top = left = 1 midX = width +1 midY = height +1 BASE_COLORS = ["red","green","blue"] def posterize(pic): # simplifies the color range for pixel in getPixels(pic): for color in BASE_COLORS: level = 0 if getColor(color, pixel ) > 127: level = 255 setColor(color, level) return pic def grayscale(gPic): #sets RGB values equal to one another for p in getPixels(gPic): intensity = (getRed(p) + getGreen(p) + getBlue(p))/3 setColor(p, makeColor(intensity, intensity, intensity)) return gPic def negative(nPic): # makes all the colors opposite original values for px in getPixels(nPic): inverse_rgb = [255 - getColor(color, px) for color in BASE_COLORS ] setColor(px, makeColor(*inverse_rgb) ) return nPic def sunset(sPic): # decreases blue and green to add a warmer red tone for p in getPixels(sPic): for color in ["blue","green"]: setColor( getColor( color, p ) * 0.07 ) return sPic def copyPicture(littlePic, bg, x, y): for destX in range(getWidth(littlePic)): for destY in range(getHeight(littlePic)): fromPixel = getPixel(littlePic, destX+1, destY+1) destPixel = getPixel(bg, destX+x, destY+y) setColor(destPixel, getColor(fromPixel)) copyPicture(posterize(topLeft), canvas, left, top) copyPicture(grayscale(topRight), canvas, midX, top) copyPicture(negative(bottomLeft), canvas, left, midY) copyPicture(sunset(bottomRight), canvas, midX, midY) copyPicture(middlePicture, canvas, midX/2, midY/2) return canvas
Daniel K
September 30, 2007, September 30, 2007 11:21, permalink
You could also achieve the same thing as the last refactoring using a list of getters.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17
getters = [getRed,getGreen,getBlue] setters = [setRed,setGreen,setBlue] def posterize(pic): # simplifies the color range for pixel in getPixels(pic): for (getter,setter) in zip(getters,setters): level = 0 if getter( pixel ) > 127: level = 255 setter( pixel, level ) return pic def negative(nPic): # makes all the colors opposite original values for px in getPixels(nPic): inverse_rgb = [255 - getter( px ) for getter in getters ] setColor(px, makeColor( *inverse_rgb) ) return nPic
Silverfish
September 30, 2007, September 30, 2007 12:26, permalink
I agree with Daniel K's idea of having generic getColor and setColor functions. However, there already are functions in the original code with those names. Perhaps getBaseColor and setBaseColor for the new functions instead.
Also, I've noticed that it looks like the montage function definition seems to include the definitions of the other functions. I suspect this isn't what is intended, and the posterize (and similar functions) should be separate from the montage function, so they can be used (or tested) by other programs, and so montage doesn't have the overhead of generating new functions each time it is run. This last bit probably won't be a factor, but unless there is a reason for it, separating the functions (possibly into a different module as Darius suggests) seems like a good idea.
Another minor point, there is a convention for functions that modify their arguments (such as posterize), not to return anything.
Creates a collage