Lemme put my "code review" hat on...  there we go.  I hope this doesn't come
across as harsh, but it *is* criticism (and a couple questions).  That's what
code reviews are for, and the goal is to produce better code (and hopefully a
better coder), not to attack anyone, poke fun at them, disparage their 
parenting or personal hygiene, nor to accuse them of molesting farm animals.

All that stuff is just a bonus.  ;)

1 & 2 are potential bugs, the rest aren't that big a deal.

1) How do you know when pixelGrabber.getPixels() will return int[] vs byte[]? 
If it returns byte[], you're looking at a ClassCastException.  I've never used
a pixel grabber, so your code could be perfectly kosher, I just thought it was
worth asking.

2) How do you know the order of 'colorants' in the integer?  Your code assumes
it to be in the high 8 bits of the int.  Is this guaranteed?

3) createAlphaMask contains a mix of tabs and spaces.  No Bueno.  Using nothing
but tabs is tolerable, but different editors *will* use different widths for
tabs.  Spaces will always be displayed consistently.  Spaces good.  Tabs bad.  
;)

4) I'm surprised you needed to fully qualify java.awt.image.PixelGrabber and
java.awt.image.ImageObserver.ABORT.  ImageObserver in particular is specifically
imported by name.

5) The use of Logger.  createAlphaMask is the only place it's used inside this
particular file.  I checked several Important Classes (PdfReader, PdfWriter,
PdfContentByte...) and didn't see it used anywhere.  My private branch of
paulo155 doesn't use it at all.  It's not much use if no one knows to look for 
it. 

Which segues nicely into...

6) No javadoc.  Neither do any of the other private methods in the
'implementation' section, but it could be used to clue people into the whole
logger thing, removing at least part of my objection to its use.


--Mark Storer
  PDF Guy
  Cardiff.com

#include <disclaimer>
typedef std::disclaimer<Cardiff> DisCard;


------------------------------------------------------------------------------
_______________________________________________
iText-questions mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/itext-questions

Buy the iText book: http://www.1t3xt.com/docs/book.php

Reply via email to