I am checking for particular keywords in a string, and then changing the foreground and background colors of substrings based on the keywords. Following is the snippet for checking the background code:
if(string.contains("[30;42m") || string.contains("[31;42m") || string.contains("[32;42m") ||
string.contains("[33;42m") || string.contains("[34;42m") || string.contains("[35;42m")
|| string.contains("[36;42m") || string.contains("[37;42m")){
//the parameter of indexOf and replaceAll will be one from the above
//using [30;42m as an example
index = string.indexOf("[30;42m");
string = string.replaceAll("\\[30;42m", "");
backgroundColor = Color.GREEN;
//foreground is already set using separate conditions for 30,32,32...37m
}
Is there any way that I can do something as I have shown above? Currently, I have separate conditions for each substring, which doesn't incorporates "DRY" principle. My code is working, but I am looking for a way to reduce these if
statements.
Edit: The if blocks are for setting the background and foreground colors. For example if I have "34;42" code, this means text foreground color is Blue and background color is Green, and so on.
-
3\$\begingroup\$ Could you please provide a little more conetext on the implementation inside the if-block? I strongly suspect the refactoring strategy will change, depending on how you impelemented the body. \$\endgroup\$Vogel612– Vogel6122014年05月09日 07:31:20 +00:00Commented May 9, 2014 at 7:31
3 Answers 3
You can do it a little more generic but I'm afraid you still have to test each.
I don't know if your case is always changing to green or not.
If not I suggest the following enum :
/**
*
* @author chillworld
*/
public enum ColorMapperEnum {
GREEN("[30;42m",Color.GREEN) ,
RED("[31;42m",Color.RED) ,
BLACK("[32;42m", Color.BLACK) ,
BLUE("[33;42m", Color.BLUE); // and so on
private String textToFind;
private Color targetColor;
private ColorMapperEnum(String textToFind, Color targetColor) {
this.textToFind = textToFind;
this.targetColor = targetColor;
}
public String getTextToFind() {
return textToFind;
}
public Color getTargetColor() {
return targetColor;
}
}
Now you can do a whole lot more generic in your java code :
for (ColorMapperEnum colorMapper : ColorMapperEnum.values()) {
if(string.contains(colorMapper.getTextToFind()){
index = string.indexOf(colorMapper.getTextToFind());
string = string.replaceAll("\\" + colorMapper.getTextToFind(), "");
backgroundColor = colorMapper.getTextToFind();
}
}
I did use the same naming as yo but I advice to take better naming.
String string => very bad, for what stands it?
Now I don't know if there could be a case that multiple conditions are true and what there must be done when multiple conditions are true.
An enum values()
has always the same result in the same order.
Use that in your advantage when tackling that problem.
-
\$\begingroup\$ Thank you for your feedback, yes you are right. I just need to set three values instead of two: textToFind, foregroundColor, BackgroundColor \$\endgroup\$w_ahmed– w_ahmed2014年05月09日 09:52:58 +00:00Commented May 9, 2014 at 9:52
-
\$\begingroup\$ One more thing, willn't it be better from performance viewpoint if I use a hash table? \$\endgroup\$w_ahmed– w_ahmed2014年05月09日 10:09:11 +00:00Commented May 9, 2014 at 10:09
-
\$\begingroup\$ where exactly? in stead of the enum? \$\endgroup\$chillworld– chillworld2014年05月09日 12:04:13 +00:00Commented May 9, 2014 at 12:04
-
\$\begingroup\$ Yes,instead of using enum. \$\endgroup\$w_ahmed– w_ahmed2014年05月09日 12:19:08 +00:00Commented May 9, 2014 at 12:19
-
2\$\begingroup\$ Actually Enum is better. You have to construct once. Always singleton and accesible in each class. If you need foreground also change
targetColor
tobackgroundColor
and add a color forforegroundColor
. Add the second color in the constructor and values of the enum. (if help needed ask and I refactor code here). Is it possible that 30 and 31 are in the same string? \$\endgroup\$chillworld– chillworld2014年05月09日 12:39:37 +00:00Commented May 9, 2014 at 12:39
One idea I'd have here is maybe use a different approach and parser your string into Tokens.
Have a Token each for the color markers (a simple 2-tuple/pair implementation will do) and the non-marker strings in between.
So a string such as
This[30;42m is an[31;42m Example
would become a list like (pseudocode)
String("This"), Tuple(30, 42), String(" is an"), Tuple(31, 42), String(" Example")
Now you just need to iterate over the list, compare each tuple with the set of tuples you are looking for, building a new list that you can finally turn back into a string.
I hope this is understandable.
It appears that you want to support 8 background colors, and probably 8 foreground colors as well. That would be 64 magic strings that you are looking for, with 64 cases to handle them all. With so many cases, string matching is inappropriate; you want to do pattern matching. Pattern matching should be done using regular expressions.
Furthermore, you're probably better off writing a proper ANSI terminal emulator, or at least just a state machine to support for changing colors. The following class breaks up an input string into multiple colored strings.
import java.awt.Color;
import java.util.Iterator;
import java.util.NoSuchElementException;
import java.util.regex.*;
public class ANSIAttrString implements Iterable<ANSIAttrString> {
//////////////////////////////////////////////////////////////////////
public static class TermState {
public static final TermState DEFAULT = new TermState(Color.WHITE, Color.BLACK);
public final Color fg, bg;
public TermState(Color fg, Color bg) {
this.fg = fg;
this.bg = bg;
}
public TermState(TermState init, String[] attrs) {
Color fg = init.fg;
Color bg = init.bg;
for (String attr : attrs) {
switch (attr) {
case "0": fg = DEFAULT.fg;
bg = DEFAULT.bg;
break;
case "30": fg = Color.BLACK; break;
case "31": fg = Color.RED; break;
case "32": fg = Color.GREEN; break;
case "33": fg = Color.YELLOW; break;
case "34": fg = Color.BLUE; break;
case "35": fg = Color.MAGENTA; break;
case "36": fg = Color.CYAN; break;
case "37": fg = Color.WHITE; break;
case "39": fg = DEFAULT.fg; break;
case "40": bg = Color.BLACK; break;
case "41": bg = Color.RED; break;
case "42": bg = Color.GREEN; break;
case "43": bg = Color.YELLOW; break;
case "44": bg = Color.BLUE; break;
case "45": bg = Color.MAGENTA; break;
case "46": bg = Color.CYAN; break;
case "47": bg = Color.WHITE; break;
case "49": bg = DEFAULT.bg; break;
}
}
this.fg = fg;
this.bg = bg;
}
public String toString() {
return fg + " on " + bg;
}
}
//////////////////////////////////////////////////////////////////////
private static class Iter implements Iterator<ANSIAttrString> {
private final String s;
private Matcher m;
private TermState state;
private int index;
Iter(String s, TermState init, Matcher m) {
this.s = s;
this.state = init;
this.m = m;
}
public boolean hasNext() {
return this.m != null;
}
public ANSIAttrString next() {
if (this.m == null) {
throw new NoSuchElementException();
}
int thisEnd = m.find() ? m.start() : m.regionEnd();
try {
return new ANSIAttrString(s.substring(index, thisEnd), this.state);
} finally {
if (m.hitEnd()) {
this.m = null;
} else {
String[] attrs = m.group(1).split(";");
state = new TermState(state, attrs);
this.index = m.end();
}
}
}
public void remove() {
throw new UnsupportedOperationException();
}
}
//////////////////////////////////////////////////////////////////////
public final TermState state;
public final String string;
private static final Pattern SGR_PATTERN = Pattern.compile(
"\\e\\[(\\d+(?:;\\d+)*)m"
);
public ANSIAttrString(String string) {
this(string, TermState.DEFAULT);
}
public ANSIAttrString(String string, TermState state) {
this.state = state;
this.string = string;
}
public Iterator<ANSIAttrString> iterator() {
return new Iter(this.string, this.state, SGR_PATTERN.matcher(this.string));
}
}
Test case:
public static void main(String[] args) {
String str = "It's a \u001b[31mRainbow\u001b[34;43m colored\u001b[39m world.";
for (ANSIAttrString ats : new ANSIAttrString(str)) {
System.out.println(ats.state + ": " + ats.string);
}
}
java.awt.Color[r=255,g=255,b=255] on java.awt.Color[r=0,g=0,b=0]: It's a java.awt.Color[r=255,g=0,b=0] on java.awt.Color[r=0,g=0,b=0]: Rainbow java.awt.Color[r=0,g=0,b=255] on java.awt.Color[r=255,g=255,b=0]: colored java.awt.Color[r=255,g=255,b=255] on java.awt.Color[r=255,g=255,b=0]: world.