Aa245e36dd988cb88f695c57ff86e02e

Creates a collage

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 !

2d64a68dbd49eaf5cf40fcd394f19b13

Berco Beute

September 29, 2007, September 29, 2007 02:01, permalink

No rating. Login to rate!

Just a simple enhancement (also to test out this site :) )

1
top, left = 1, 1
C63d2d886af30dbea99a341467ac116d

Darius Damalakas

September 29, 2007, September 29, 2007 03:06, permalink

No rating. Login to rate!

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

Avatar

Daniel K

September 29, 2007, September 29, 2007 11:07, permalink

No rating. Login to rate!

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
Avatar

Daniel K

September 30, 2007, September 30, 2007 11:21, permalink

No rating. Login to rate!

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
A4f5a41dda93df538647cd21df5d3552

Silverfish

September 30, 2007, September 30, 2007 12:26, permalink

No rating. Login to rate!

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.

Your refactoring





Format Copy from initial code

or Cancel