On 22:41 Wed 26 Jan , Ivan Maidanski wrote:
> Hi,
>
> It's ok but:
>
> 1. I'd better rewrote check for null (IMHO looks better):
>
> try {
> Pattern.quote(null);
> harness.check(true);
> } catch (NullPointerException e) {
> harness.check(false);
> }
>
Yeah I like this version better too. The current one reads rather oddly.
However, you seem to have inverted the logic; harness.check(true) should
be called when the NPE is given.
If an NPE should be thrown for null values, that should be documented in
the Classpath patch too.
> 2. Add a test for a string which already contains \Q
>
> Regards.
>
> Tue, 25 Jan 2011 23:30:41 +0200 Pekka Enberg <penb...@cs.helsinki.fi>:
>
> > On Mon, 2011-01-24 at 23:36 +0000, Dr Andrew John Hughes wrote:
> > > Oh this is one of Ivan's? I didn't spot that. Which number is it?
> > > (so I don't review it all over again ;-) )
> > >
> > > A test case would be great. I can't really review this patch well without
> > > knowing what it's supposed to be doing.
> >
> > Here's a test case I came up with. Ivan, anything else I should test
> > for?
> >
> > Pekka
> >
> > // Tags: JDK1.5
> >
> > // Copyright (C) 2011 Pekka Enberg
> >
> > // This file is part of Mauve.
> >
> > // Mauve is free software; you can redistribute it and/or modify
> > // it under the terms of the GNU General Public License as published by
> > // the Free Software Foundation; either version 2, or (at your option)
> > // any later version.
> >
> > // Mauve is distributed in the hope that it will be useful,
> > // but WITHOUT ANY WARRANTY; without even the implied warranty of
> > // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > // GNU General Public License for more details.
> >
> > // You should have received a copy of the GNU General Public License
> > // along with Mauve; see the file COPYING. If not, write to
> > // the Free Software Foundation, 59 Temple Place - Suite 330,
> > // Boston, MA 02111-1307, USA.
> >
> > package gnu.testlet.java.util.regex.Pattern;
> >
> > import gnu.testlet.*;
> > import java.util.regex.*;
> >
> > public class quote implements Testlet
> > {
> > private TestHarness harness;
> >
> > public void test (TestHarness harness)
> > {
> > harness.check(Pattern.quote("hello"), "\\Qhello\\E");
> >
> > boolean pass = false;
> > pass = false;
> > try
> > {
> > Pattern.quote(null);
> > }
> > catch (NullPointerException e)
> > {
> > pass = true;
> > }
> > harness.check(pass);
> > }
> > }
>
>
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint = F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
