On Tue, 2008-04-15 at 22:49 +0000, Vreixo Formoso Lopes wrote:
> +public class SVGSurface extends Surface
>
Yup.
> + public static final Status WRITE_ERROR = new ...
That's fine, of course. When a Constant is needed it's needed.
I usually try to do all the Constants when adding something, but
sometimes when it's a really long list of things that for which I don't
know what they all do, I do the ones I can figure out / am using.
You came across a case of needing that Constant, so add it, no question.
> - public void writeToPNG(String filename) {
> + public void writeToPNG(String filename) throws IOException {
That's fine, but maybe do the access check _before_ calling Cairo? ie
File's canWrite()
>
> - surface.writeToPNG(OUTPUT_FILENAME);
> + try {
> + surface.writeToPNG(OUTPUT_FILENAME);
> + } catch (IOException e) {
> + fail("Unexpected file write error");
> + }
>
That's fine, but we should probably improve the
> public final void testImageSurfaceWriteToPNG() {
test to actually verify this behaviour.
In other words, a better test would be to try writing to a known
unwritable path and test that the exception _does_ happen.
Then test writing to a path that known to work (ie, tmp/tests/) and make
sure that the exception does _not_ happen.
[That test case class is still rather a work in progress. What I wanted
to do was draw something and then positively test that the correct image
was drawn, but that's a bit of a tough proposition]
AfC
Sydney
--
Andrew Frederick Cowie
Operational Dynamics is an operations and engineering consultancy
focusing on IT strategy, organizational architecture, systems
review, and effective procedures for change management. We actively
carry out research and development in these areas on behalf of our
clients, and enable successful use of open source in their mission
critical enterprises, worldwide.
http://www.operationaldynamics.com/
Sydney New York Toronto London
|
