Muhammad Ali wrote:
Hi,
This is my first attempt at python. I tried my hand at steganography for
fun. The code includes separate, combine, encode, decode functions. The
separate function takes a number and it's base and returns a list containing
each digit of the number in a list, vice versa for combine. The encode
function encodes each char's ascii value into each pixel's RGB values at the
least significant end. The decode function does the opposite. e.g.,
a = 97, so if we have at a pixel r, g, b (230, 107, 155) it will become
(230, 109, 157) when it's encoded.
Since I am new and don't speak the python idiom yet, I would like the
experts to pythonify the following code as much as possible for both python
2.6 and 3.0. Any other suggestion to improve the quality would also be
highly appreciated
Thanks a lot!
#The code starts here:
def separate(num, base):
li = []
while num / base > 0:
li.insert(0, num % base)
num = num / base
li.insert(0,num)
return li
def combine(tup, base):
num = 0
mul = pow(base, len(tup) - 1)
for i in tup:
num = num + i * mul
mul = mul / base
return num
#Will make changes to the original image and not to a copy! You have been
warned!
import Image
def encode(img, text):
x = 0
y = 0
height, width = img.size
text = text + "~~"
if len(text) > height * width:
return false
pix = img.load()
for c in text:
li = separate(ord(c), 10)
if(len(li) == 1):
li.insert(0, 0)
li.insert(0, 0)
elif(len(li) == 2):
li.insert(0, 0)
r, g, b = pix[x, y]
r = r - (r % 10) + li[0]
if (r > 255):
r = r - 10
g = g - (g % 10) + li[1]
if (g > 255):
g = g - 10
b = b - (b % 10) + li[2]
if (b > 255):
b = b - 10
pix[x,y] = (r,g,b)
if y == width - 1:
y = 0
x = x + 1
else:
y = y + 1
img.save(img.filename)
def decode(img):
x = 0
y = 0
text = ""
c = ""
height, width = img.size
pix = img.load()
while 1:
r, g, b = pix[x, y]
if (c == '~') and chr(combine([r % 10, g % 10, b % 10], 10)) == '~':
return text[:len(text) - 1]
c = chr(combineUnits([r % 10, g % 10, b % 10], 10))
text = text + c
if y == width - 1:
y = 0
x = x + 1
else:
y = y + 1
Pretty good for your first Python code. Clearly you have some
experience in other language(s). Comments follow, no particular order.
Import should be at begin of module, unless it's just plain not
possible. Functions should have doc-strings, explaining what they do,
and what the arguments look like. For example, first two functions
wouldn't work with floats, only ints. There also should be comments
stating what ranges of values are legal. For example, num is a positive
integer, and base is an integer between one and 250 or so.
separate() could be simplified and optimized, some depending on the
range of expected parameters, but below we'll see that it should be
replaced entirely. So don't optimize prematurely.
separate() and combine() both naturally want to do the digits in the
oppposite order, and nothing else cares. But again, it's premature
optimization.
Looking at encode(), we see that the only call to separate() is with 10
as a base. And it immediately takes the nicely formatted number, and
pads it to 3 digits. Sounds like a standard formatting operation to me,
so why not just use the format method with a 3 for width? Then loop
over the string, or even use a list comprehension on it.
encode() refers to }false", but the built-in value is called "False"
encode() uses repetitive code to combine two tuple/lists. Ideal for a
separate function, perhaps using zip(). Then we could replace 14 lines
with:
pix[x, y] =pixel(pix[x, y], li)
in encode(), row/column incrementing could be simplified, and common
case sped up:
y += 1
if y == width: y, x = 0, x+1
Incidentally, aren't you processing the pixels in column-major order?
Doesn't matter much for tiny images, but for symmetric designs, it's
usually better to keep memory accesses localized, to reduce swapping. I
could be all wet here, though, just going on memory.
decode() refers to combineUnits() in one of its two calls, but it's
actually called combine(). Further, there's no need to call it twice.
Better would be to have two "c" variables, maybe calling one of them 'lastc'
lastc = c
c = chr(combineUnits([r % 10, g % 10, b % 10], 10))
if (c == '~') and lstc == '~':
return text[:len(text) - 1]
In decode(), while 1 is usually spelled: while True
Hope these help.
DaveA
_______________________________________________
Tutor maillist - Tutor@python.org
http://mail.python.org/mailman/listinfo/tutor
