13
\$\begingroup\$

Description

Match a Simple Sentence or a partial sentence Suitable for matching

  • People Names (to some extent)
  • Product Titles (to some extent)
  • Correct use of apostrophe (to some extent)

Input/Result

match("Because I'm Batman")                                 true
match("John Cena's fans")                                   true
match("Nice.")                                              true
match("Mat's Mug")                                          true
match("Good Boy")                                           true
match("Assert True.")                                       true
match("The students' projects")                             true
match("The Johnsons' house is on fire.")                    true
match("Tim's and Marty's ice cream")                        true
match("You're late.")                                       true
match("Cat's eyes are blue.")                               true
match("B.A.T.")                                             true
match("B. A. T.")                                           true
match(" Hello World ")                                      true
match(" K.O. ")                                             true
match(" Js' friend ")                                       true
match(" J A V A ")                                          true
match("I'll look into it")                                  true
match("I can't beleive it.")                                true
match("The students'")                                      false
match("The  students")                                      false
match(".")                                                  false
match("")                                                   false
match("AA")                                                 false
match("A")                                                  false
match("B..A.T.")                                            false
match("You''re late.")                                      false
match("' ; delete from users ;")                            false
match("abcdefg@")                                           false
match("Bhathiya & JaDogg are the same")                     false
match("Interesting !")                                      false
match("B..A.T.!@#$!@")                                      false
match("null")                                               false
match("The# students'")                                     false
match("The  students  are bad")                             false
match("aaaa#")                                              false
match("abc$")                                               false
match("xyz%")                                               false
match("@@@@@@@")                                            false
match(".........")                                          false
match("I'm happy..")                                        false
match("Why?")                                               false
match("Gim'me")                                             false
match("weird4431")                                          false
match("1234123")                                            false
match(""hello"")                                            false
match("'hello'")                                            false

Other

  • Compiled with jdk1.8.0_05
  • Source is in Java 1.6
  • JUnit4 is used for Unit Tests

Code

This code can also be downloaded from GitHub

SimpleSentence.java: (34 lines, 971 bytes)

/**
 * Match a Simple Sentence or a partial sentence<br />
 * Suitable for matching
 * <ul>
 * <li>People Names (to some extent)</li>
 * <li>Product Titles (to some extent)</li>
 * <li>Correct use of apostrophe (to some extent)</li>
 * </ul>
 *
 * @author Bhathiya
 */
public class SimpleSentence {

    private static final String SIMPLE_SENTENCE
            = "([a-zA-Z]+(\\.|\\. |'(s |re |t |m |ll )|s' | )?)+";
    private static final Pattern SIMPLE_SENTENCE_PATTERN = Pattern.compile(
            SIMPLE_SENTENCE);

    public static boolean match(String toTest) {
        if (toTest == null) {
            return false;
        }

        String trimmed = toTest.trim();

        return (trimmed.length() > 2 
                && !trimmed.startsWith("'") && !trimmed.endsWith("'")
                && SIMPLE_SENTENCE_PATTERN.matcher(trimmed).matches());
    }
}

Unit Test

SimpleSentenceTest.java: (74 lines, 2666 bytes)

/**
 * Unit Test for SimpleSentence.java
 *
 * @author Bhathiya
 */
public class SimpleSentenceTest {

    public static boolean match(String strToTest) {
        boolean matches = SimpleSentence.match(strToTest);
        System.out.printf("%-60s%s\n",
                String.format("match(\"%s\")", strToTest),
                matches ? "true" : "false");
        return matches;
    }

    @Test
    public void testMatching() {
        assertTrue(match("Because I'm Batman"));
        assertTrue(match("John Cena's fans"));
        assertTrue(match("Nice."));
        assertTrue(match("Mat's Mug"));
        assertTrue(match("Good Boy"));
        assertTrue(match("Assert True."));
        assertTrue(match("The students' projects"));
        assertTrue(match("The Johnsons' house is on fire."));
        assertTrue(match("Tim's and Marty's ice cream"));
        assertTrue(match("You're late."));
        assertTrue(match("Cat's eyes are blue."));
        assertTrue(match("B.A.T."));
        assertTrue(match("B. A. T."));
        assertTrue(match(" Hello World "));
        assertTrue(match(" K.O. "));
        assertTrue(match(" Js' friend "));
        assertTrue(match(" J A V A "));
        assertTrue(match("I'll look into it"));
        assertTrue(match("I can't beleive it."));
    }

    @Test
    public void testNotMatching() {
        assertFalse(match("The students'"));
        assertFalse(match("The  students")); //extra space inside
        assertFalse(match("."));
        assertFalse(match(""));
        assertFalse(match("AA"));
        assertFalse(match("A"));
        assertFalse(match("B..A.T."));
        assertFalse(match("You''re late."));
        assertFalse(match("' ; delete from users ;"));
        assertFalse(match("abcdefg@"));
        assertFalse(match("Bhathiya & JaDogg are the same"));
        assertFalse(match("Interesting !"));
        assertFalse(match("B..A.T.!@#$!@"));
        assertFalse(match(null));
        assertFalse(match("The# students'"));
        assertFalse(match("The  students  are bad")); //extra spaces inside
        assertFalse(match("aaaa#"));
        assertFalse(match("abc$"));
        assertFalse(match("xyz%"));
        assertFalse(match("@@@@@@@"));
        assertFalse(match("........."));
        assertFalse(match("I'm happy.."));
        assertFalse(match("Why?"));
        assertFalse(match("Gim'me"));
        assertFalse(match("weird4431"));
        assertFalse(match("1234123"));
        assertFalse(match("\"hello\""));
        assertFalse(match("'hello'"));
    }
}

Project Object Model

pom.xml: (23 lines, 917 bytes)

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" 
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>
    <groupId>info.simpll</groupId>
    <artifactId>SimpleSentence</artifactId>
    <version>0.1-SNAPSHOT</version>
    <packaging>jar</packaging>
    <dependencies>
        <dependency>
            <groupId>junit</groupId>
            <artifactId>junit</artifactId>
            <version>4.10</version>
            <scope>test</scope>
        </dependency>
    </dependencies>
    <properties>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
        <maven.compiler.source>1.6</maven.compiler.source>
        <maven.compiler.target>1.6</maven.compiler.target>
    </properties>
</project>

Questions

  • Is there a better way to write the regex or match() ?
  • Does the given code follow best practices?
  • Does the unit test code follow best practices?
\$\endgroup\$
3
  • 1
    \$\begingroup\$ note: pom.xml is auto generated by netbeans 8.0 \$\endgroup\$
    – JaDogg
    Commented Aug 16, 2014 at 22:58
  • 3
    \$\begingroup\$ assertTrue(match("Mat's Mug")); - hi! \$\endgroup\$ Commented Aug 17, 2014 at 3:52
  • 1
    \$\begingroup\$ I'm really sorry to disturb you after so much time, but The students' is a perfectly valid sentence. Otherwise, Mat's Mug isn't a sentence either. Grammatically, students' is the plural of student's, which means that something belongs to the students. \$\endgroup\$ Commented Jan 16, 2015 at 18:58

3 Answers 3

15
\$\begingroup\$

There are a number of things that could be improved here.

You're doing the right thing with the pre-compiled regular expression/pattern, but, you have fallen victim to the little-known auto-format-muck-up-monster, and what I consider the magic-value-overcompensation issue:

private static final String SIMPLE_SENTENCE
        = "([a-zA-Z]+(\\.|\\. |'(s |re |t |m |ll )|s' | )?)+";
private static final Pattern SIMPLE_SENTENCE_PATTERN = Pattern.compile(
        SIMPLE_SENTENCE);
  • the magic-value-overcompensation-issue is that you have declared a constant, to preserve a magic value, that is only used once, in another magic value.
  • the auto-format-muck-up-monster is the location of the line-break in the Pattern. In this case, it is more readable with the Pattern starting on the new line.

Your code would look much simpler with just:

private static final Pattern SIMPLE_SENTENCE_PATTERN =
        Pattern.compile("([a-zA-Z]+(\\.|\\. |'(s |re |t |m |ll )|s' | )?)+");

Now, the first thing you do in your match method (well, the second thing), is trim the string.

This is a relatively expensive operation because it makes a copy of the data, and a new String and char[] array instance. It would be much simpler to just incorporate the white-space in to the pattern... add \\s* at the beginning and end.

Similarly, you do not allow the value to start with, or end with, a single quote ', so incorporate that in to your pattern too. Then you may as well make the null-check a condition on the return, ending up with the code:

private static final Pattern SIMPLE_SENTENCE_PATTERN =
     Pattern.compile("\\s*(?!')([a-zA-Z]+(\\.|\\. |'(s |re |t |m |ll )|s' | )?)+(?!')\\s*");

public static boolean match(String toTest) {
    return toTest != null && toTest.length() > 2
           && SIMPLE_SENTENCE_PATTERN.matcher(toTest).matches();
}

Now, about the unit tests....

I would suggest a couple of arrays that contain passing, and failing values. Something simple like:

private static final String[] passingValues = {
    "Because I'm Batman",
    "John Cena's fans",
    "Nice.",
    "Mat's Mug",
    "Good Boy",
    ....
};

and then the test is:

@Test
public void testMatching() {
    for (String val : passingValues) {
        assertTrue(match(val));
    }
}

Adding new test cases is much simpler this way, and the DRY-factor is high.

\$\endgroup\$
0
11
\$\begingroup\$

A few points on top of @rolfl's.

The ternary with the boolean variable matches is a bit silly:

System.out.printf("%-60s%s\n",
        String.format("match(\"%s\")", strToTest),
        matches ? "true" : "false");

This works just as well:

System.out.printf("%-60s%s\n",
        String.format("match(\"%s\")", strToTest),
        matches);

Using the message parameters of assertions

When you have a bunch of assertions like this:

assertTrue(match("John Cena's fans"));
assertTrue(match("Nice."));
assertTrue(match("Mat's Mug"));
assertTrue(match("Good Boy"));
assertTrue(match("Assert True."));

An unfortunate thing is that when something fails, it's not super-easy to find which one it was. The message parameter of assertions can help, for example:

assertTrue("should pass: Nice.", match("Nice."));

This way if the assertion on Nice. fails, the message will appear in the error message, so it will be clear immediately which assertion of the many failed.

Of course, duplicating the value under test inside the message would be silly, this is another reason why you need a helper method like @rolfl proposed. Here's another alternative:

private void testMatching(String... passingValues) {
    for (String val : passingValues) {
        assertTrue("doesn't match: " + val, match(val));
    }
}

@Test
public void testMatching() {
    testMatching(
            "Because I'm Batman",
            "John Cena's fans"
    );
}
\$\endgroup\$
6
\$\begingroup\$

Consider parameterized testing?

JUnit

Here is how you can use JUnit 4's Parameterized runner with your test cases so that you can get a slightly nicer output in Eclipse. The gotcha is that you need to annotate your test class with @RunWith using the Parameterized runner, and it looks like you can only 'parameterize' for one class constructor.

package com.se;

import static org.junit.Assert.assertEquals;

import java.util.Arrays;
import java.util.Collection;
import java.util.regex.Pattern;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;

@RunWith(Parameterized.class)
public class MatcherTest {
    private static final Pattern SIMPLE_SENTENCE_PATTERN = Pattern.compile("\\s*(?!')([a-zA-Z]+(\\.|\\. |'(s |re |t |m |ll )|s' | )?)+(?!')\\s*");

    private String testString;
    private Boolean results;

    public MatcherTest(final String testString, final Boolean results) {
        this.testString = testString;
        this.results = results;
    }

    public static boolean match(final String toTest) {
        return toTest != null && toTest.length() > 2 && SIMPLE_SENTENCE_PATTERN.matcher(toTest).matches();
    }
    
    public static Object[][] getTestDataForAll() {
        return new Object[][] { 
                { "Because I'm Batman", Boolean.TRUE }, 
                { "John Cena's fans", Boolean.TRUE }, 
                { "Nice.", Boolean.TRUE },
                { "The students'", Boolean.FALSE }, 
                { "The  students", Boolean.FALSE }, 
                { ".", Boolean.FALSE } };
    }
    
    @Parameters
    public static Collection<Object[]> getTestDataForJUnit() {
        return Arrays.asList(getTestDataForAll());
    }

    @Test
    public void junitTest() {
        assertEquals(match(testString), results);
    }
}

JUnit test output in Eclipse

TestNG

Eclipse's TestNG plug-in has an even better display of test parameters and results. The implementation is simpler too using @DataProvider method annotations, and you can specify multiple providers for multiple methods. Note: getTestDataForAll() is the same as the snippet for JUnit example above.

package com.se;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
    
import java.util.Arrays;
import java.util.Collection;
import java.util.regex.Pattern;

import org.testng.annotations.Test;
import org.testng.annotations.DataProvider;

public class MatcherTest {
    private static final Pattern SIMPLE_SENTENCE_PATTERN = Pattern.compile("\\s*(?!')([a-zA-Z]+(\\.|\\. |'(s |re |t |m |ll )|s' | )?)+(?!')\\s*");

    public static boolean match(final String toTest) {
        return toTest != null && toTest.length() > 2 && SIMPLE_SENTENCE_PATTERN.matcher(toTest).matches();
    }
    
    @DataProvider(name = "allDataProvider")
    public static Object[][] getTestDataForAll() {
        return new Object[][] { 
                { "Because I'm Batman", Boolean.TRUE }, 
                { "John Cena's fans", Boolean.TRUE }, 
                { "Nice.", Boolean.TRUE },
                { "The students'", Boolean.FALSE }, 
                { "The  students", Boolean.FALSE }, 
                { ".", Boolean.FALSE } };
    }
    
    private static String[] matches = { "Because I'm Batman", "John Cena's fans", "Nice." };
    private static String[] nonMatches = { "The students'", "The  students", "." };

    @DataProvider(name = "matchingDataProvider")
    public static Object[][] getMatchingTestDataForTestNG() {
        final Object[][] results = new Object[matches.length][1];
        int i = 0;
        for (final String match : matches) {
            results[i++] = new Object[] { match };
        }
        return results;
    }

    @DataProvider(name = "nonMatchingDataProvider")
    public static Object[][] getNonMatchingTestDataForTestNG() {
        final Object[][] results = new Object[nonMatches.length][1];
        int i = 0;
        for (final String nonMatch : nonMatches) {
            results[i++] = new Object[] { nonMatch };
        }
        return results;
    }

    @Test(dataProvider = "matchingDataProvider")
    public static void testNgTestForMatch(final String testString) {
        assertThat("String matches", match(testString));
    }

    @Test(dataProvider = "nonMatchingDataProvider")
    public static void testNgTestForNonMatch(final String testString) {
        assertThat("String does not match", !match(testString));
    }

    @Test(dataProvider = "allDataProvider")
    public static void testNgTestForAll(final String testString, final Boolean results) {
        assertThat(Boolean.valueOf(match(testString)), equalTo(results));
    }
}

TestNG test output in Eclipse

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.