6
\$\begingroup\$

I am going through the CodingBat exercises for Java. I got to this problem:

Return a version of the given string, where for every star (*) in the string the star and the chars immediately to its left and right (if any) are gone. So "ad", "ab*cd", "ab**cd", "*ead", and "ade*" all yield "ad".

I decided to solve this using regular expressions. Here is my code:

public String starOut(String str){

    String s = " " + str + " "; //Avoiding OOB exceptions.
    String n = ""; //Used for replacements of s.

    if (s.contains("***")) {
        n = s.replaceAll(".[*][*][*].", "");
        s = n;
    }

    if (s.contains("**")) {
        n = s.replaceAll(".[*][*].", "");
        s = n;
    }

    if (s.contains("*")) {          
        n = s.replaceAll(".[*].", "");
    }

    String theOne = n.replaceAll("\\s", ""); //Remove whitespace created by s declaration.
    return theOne;
}

My code is inefficient and repetitious, and does not account for situations of a string containing more than three * adjacent to each other. I can't help feeling like I'm missing something obvious of regex that would be a beautifully logical solution.

What would be a good solution to ensure my code utilises regex in an efficient and sensical way? Would it be more appropriate to solve this by looping through characters in the original string?

\$\endgroup\$
6
\$\begingroup\$

Regular expressions are the right tool for this sort of problem, and your suspicions that your code is not great, is about right.... there's the + operator in regular expressions which will do what you want much more concisely. + matches 1-or-more times.

Consider the simple expression:

String compact = raw.replaceAll(".?\\*+.?", "");

That replaces all something-stars-something patterns with nothing.

Note, this pattern will have the following results:

ab*cd     ad
ab***cd   ad
a**b      
ab****    a

etc.

The way the expression works is as follows:

Key features of the regex are:

  • \\*+ - * is normally a special character. We have to escape it with \\ to make it a normal *. The + is a 1-or-many match. What does this mean? It means that the expression \\*+ will match at least one * character, perhaps many of them in a row.
  • .? this is a non-greedy 0-or-1 match of any character. This requires some explaining. This will match at most 1 character, but, if the overall pattern will fail to match something, then the pattern can be tried again but matching nothing. What it means, is: "if possible, match 1 character - any character (including *)"

Putting them together, you get an expression that matches any character, if there is one, before an asterisk, as well as the asterisk, and any other asterisks that follow it, and finally any other character, if there is one.

See this in an ideone here: http://ideone.com/6yMSvx

\$\endgroup\$
  • 1
    \$\begingroup\$ It won't work with abc* which should output ab (adding a ? after each . will fix it) \$\endgroup\$ – ratchet freak Apr 8 '15 at 16:30
  • \$\begingroup\$ @ratchetfreak it will work, because the regex will backtrack, and the last '.' will match the last '*' ... here, in ideone: ideone.com/6yMSvx \$\endgroup\$ – rolfl Apr 8 '15 at 16:33
  • \$\begingroup\$ check the last line of the output in your ideone... \$\endgroup\$ – ratchet freak Apr 8 '15 at 16:34
  • \$\begingroup\$ @ratchetfreak - but that is surely by design too? (which is why I added that case). There is no character to the right, nothing to replace. \$\endgroup\$ – rolfl Apr 8 '15 at 16:35
  • 1
    \$\begingroup\$ @alanbuchanan - edited my answer \$\endgroup\$ – rolfl Apr 8 '15 at 17:17
1
\$\begingroup\$

A nice solution has already been given, but some details may need some more critique.

String s = " " + str + " "; //Avoiding OOB exceptions.

There won't be any as String.replaceAll is safe in this respect.

But you want to ensure that there's something to match, which isn't a bad idea. However,

String theOne = n.replaceAll("\\s", ""); //Remove whitespace created by s declaration.

removes all whitespace, even such contained in the original string. Using String.trim would be better, but still wrong. I can't see any simple solution.


String n = ""; //Used for replacements of s.

No... 1. you don't need it. 2. even if you did, there's no reason to define it outside of the ifs. 3. and there's no reason to initialize it (hey, it's Java, no bugs due to uninitialized locals as the compiler tells you).

if (s.contains("***")) {

What about four stars?

You don't have to guard the replacing, if there's nothing found, then nothing gets replaced.

n = s.replaceAll(".[*][*][*].", "");
s = n;

This may be fine for debugging, but in the resulting code there should be just a direct assignment.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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