3
\$\begingroup\$

I have this method that creates book objects. I pass a list of strings (size 60000+) that contain the information of the book, the function then goes and extracts the information makes the book object and adds to a global list.

This solution works, but is not memory that efficient and slow due to the replacements and regex that happen each loop and strings being made. So, can I make improvements to my regex and is there a better way perform many replacements per loop

    private static void defineBooks(List<String> paragraphs) {
    // Pattern id = Pattern.compile("[^\\s]\\d+");
    Pattern title = Pattern.compile("[\\S\\s]+");
    Pattern author = Pattern.compile(", by(.*)(?=\\s*)");
    Pattern subtitle = Pattern.compile("\\[Subtitle: [^\\[\\]]+]");
    Pattern language = Pattern.compile("\\[Language: [^\\[\\]]+]");
    Pattern contents = Pattern.compile("\\[Contents: [^\\[\\]]+]");
    Pattern tempx = Pattern.compile("\\d{1,5}$");
    Pattern tempy = Pattern.compile("^\\d{1,5}|\\d{1,5}[A-Z]?\\s?$");
    Pattern i = Pattern.compile("^, by ");
    Pattern j = Pattern.compile("\\s+(?<= )\\d+(\\w)?$|(?<= )\\d+(\\w)?\\s+$|(?<=\\D)\\d+(\\w)?$");

    for (int i1 = 0, paragraphsSize = paragraphs.size(); i1 < paragraphsSize; i1++) {
        String para = paragraphs.get(i1);
        Matcher mid = tempx.matcher(para.replaceAll("\n.+", ""));
        Matcher midy = tempy.matcher(para.replaceAll("\n.+", ""));
        Matcher mtitle = title.matcher(para);
        Matcher mauthor = author.matcher(para);
        Matcher msubtitle = subtitle.matcher(para);
        Matcher mlanguage = language.matcher(para);
        Matcher mcontents = contents.matcher(para);

        Book book = new Book();

        Matcher temp = Pattern.compile("@\\d{1,5}[A-Z]?@?")
                .matcher(para.replaceAll("\\s{2,}", "@"));

        if (temp.find()) {
            book.setId(temp.group().replaceAll("@", ""));
        } else if (mid.find()) {
            book.setId(mid.group().trim());
        } else if (midy.find()) {
            book.setId(midy.group().trim());
        }
        if (mtitle.find()) {
            book.setTitle(para.replaceAll("\\[[^\\[]+(?:])", "")
                    .replaceAll("(Passed | by|, by).*+", "")
                    .replaceAll("\\s{2,}\\d{1,5}(\\s)?", "")
                    .replaceAll("\n", " ")
                    .replaceAll(",$", "")
                    .replaceAll("\\s+", " ").trim());
        }
        if (mauthor.find()) {
            String bauthor = i.matcher(mauthor.group()).replaceAll("");
            book.setAuthor(j.matcher(bauthor).replaceAll(""));
        }
        if (msubtitle.find()) {
            book.setSubtitle(msubtitle.group()
                    .replaceAll("\\[Subtitle: ", "")
                    .replaceAll("]", "")
                    .replaceAll("\\s{2,}", " "));
        } else {
            book.setSubtitle("");
        }
        if (mcontents.find()) {
            book.setContents(mcontents.group()
                    .replaceAll("\\[Contents: ", "")
                    .replaceAll("]", "")
                    .replaceAll("\\s{2,}", " "));
        } else {
            book.setContents("");
        }
        if (mlanguage.find()) {
            book.setLanguage(mlanguage.group()
                    .replaceAll("\\[Language: ", "")
                    .replaceAll("]", "")
                    .replaceAll("\\s{2,}", " "));
        } else {
            book.setLanguage("English");
        }

        if (!Objects.equals(book.getId(), "")) {
            books.add(book);
        }
    }

Book

public class Book {
    private String id;
    private String title;
    private String author;
    private String subtitle;
    private String contents;
    private String language;

    public void setId(String id) {
        this.id = id;
    }

    public void setTitle(String title) {
        this.title = title;
    }

    public void setAuthor(String author) {
        this.author = author;
    }

    public void setSubtitle(String subtitle) {
        this.subtitle = subtitle;
    }

    public void setContents(String contents) {
        this.contents = contents;
    }

    public String getContents() {
        return contents;
    }

    public void setLanguage(String language) {
        this.language = language;
    }

    public String getId() {
        return id;
    }
}
```
\$\endgroup\$
2
\$\begingroup\$

You can check the Flyweight Pattern if you have duplicates in the data. This pattern uses a cache to prevent the recalculation of similar objects.

Here is some general recommendation for your code; those changes won’t help for the performance issue, but will help when reading the code and make it shorter.

1) Extract the java.util.regex.Pattern#compile(java.lang.String) into constants; so it will be loaded only once per ClassLoader, instead of each time you call the function.

public static final Pattern TITLE = Pattern.compile("[\\S\\s]+");
public static final Pattern AUTHOR = Pattern.compile(", by(.*)(?=\\s*)");
public static final Pattern SUBTITLE = Pattern.compile("\\[Subtitle: [^\\[\\]]+]");
public static final Pattern LANGUAGE = Pattern.compile("\\[Language: [^\\[\\]]+]");
public static final Pattern CONTENTS = Pattern.compile("\\[Contents: [^\\[\\]]+]");
public static final Pattern TEMPX = Pattern.compile("\\d{1,5}$");
public static final Pattern TEMPY = Pattern.compile("^\\d{1,5}|\\d{1,5}[A-Z]?\\s?$");
public static final Pattern I = Pattern.compile("^, by ");
public static final Pattern J = Pattern.compile("\\s+(?<= )\\d+(\\w)?$|(?<= )\\d+(\\w)?\\s+$|(?<=\\D)\\d+(\\w)?$");
public static final Pattern ID = Pattern.compile("[^\\s]\\d+");

2) Instead of using a for with index, I suggest a for-each.

for (String para : paragraphs) {
   //[...]
}

3) For the two sections that don't have an else (Title & Author), I suggest that you extract them in methods.

private static void defineBooks(List<String> paragraphs) {
   //[...]
   if (mtitle.find()) {
     replaceTitle(para, book);
  }

  if (mauthor.find()) {
     replaceAuthor(mauthor, book);
  }
   //[...]
}

private static void replaceAuthor(Matcher mauthor, Book book) {
 String bauthor = I.matcher(mauthor.group()).replaceAll("");
 book.setAuthor(J.matcher(bauthor).replaceAll(""));
}

private static void replaceTitle(String para, Book book) {
 book.setTitle(para.replaceAll("\\[[^\\[]+(?:])", "")
  .replaceAll("(Passed | by|, by).*+", "")
  .replaceAll("\\s{2,}\\d{1,5}(\\s)?", "")
  .replaceAll("\n", " ")
  .replaceAll(",$", "")
  .replaceAll("\\s+", " ").trim());
}

4) For the rest, you can make methods that return a String in all cases.

private static void defineBooks(List<String> paragraphs) {
   //[...]
   book.setSubtitle(parseSubTitle(msubtitle, book));
   book.setContents(parseContents(mcontents));
   book.setLanguage(parseLanguage(mlanguage));
   //[...]
}
private static String parseLanguage(Matcher mlanguage) {
 if(mlanguage.find()) {
   return mlanguage.group()
   .replaceAll("\\[Language: ", "")
   .replaceAll("]", "")
   .replaceAll("\\s{2,}", " ");
 } else {
   return "English";
 }
}

private static String parseContents(Matcher mcontents) {
 if(mcontents.find()) {
   return mcontents.group()
   .replaceAll("\\[Contents: ", "")
   .replaceAll("]", "")
   .replaceAll("\\s{2,}", " ");
 } else {
   return "";
 }
}

private static String parseSubTitle(Matcher msubtitle, Book book) {
 if (msubtitle.find()) {
   return msubtitle.group()
   .replaceAll("\\[Subtitle: ", "")
   .replaceAll("]", "")
   .replaceAll("\\s{2,}", " ");
 } else {
   return "";
 }
}
```
|improve this answer|||||
\$\endgroup\$
1
\$\begingroup\$

To avoid having to compile the regular expressions again and again, you can create a little helper class:

public class Patterns {
    private final Map<String, Matcher> matchers = new HashMap<>();

    public Matcher compile(String pattern) {
        return matchers.computeIfAbsent(pattern, any -> Pattern.compile(pattern).matcher(""));
    }

    public String replaceAll(String s, String pattern, String replacement) {
        Matcher m = compile(pattern);
        m.reset(s);
        return m.replaceAll(replacement);
    }
}

This class compiles the regular expressions once. It allows you to use them many times in a convenient way:

Patterns pats = new Patterns();
String replaced = pats.replaceAll("text", "..", "two letters");

If that's still not enough, you can use replace instead of replaceAll, which (despite its name) also replaces all occurrences, it just operates on simple strings and since Java 9 is implemented efficiently.

|improve this answer|||||
\$\endgroup\$
1
\$\begingroup\$

I have changed my code to use substring and indexOf to extract the text from each string, which has resulted in a 30% performance increase. The memory usage is still on the high end but the method is on its own thread so there is no noticeable performance drop when the gc runs.

Before (With subtitle example)

private static void defineBooks(List<String> paragraphs) {
    ...
Pattern subtitle = Pattern.compile("\\[Subtitle: [^\\[\\]]+]");
    ...
for (int i1 = 0, paragraphsSize = paragraphs.size(); i1 < paragraphsSize; i1++) {
    ...
    Matcher msubtitle = subtitle.matcher(para);
    //...

    Book book = new Book();

    ... 
    //Old subtitle code

    if (msubtitle.find()) {
        book.setSubtitle(msubtitle.group()
                .replaceAll("\\[Subtitle: ", "")
                .replaceAll("]", "")
                .replaceAll("\\s{2,}", " "));
    } else {
        book.setSubtitle("");
    }

    ...

    if (!Objects.equals(book.getId(), "")) {
        books.add(book);
    }
}

After (along with Doi9t's answer)

private static void defineBooks(List<String> paragraphs) {
    for (String para : paragraphs) {
        ...
        Book book = new Book();
        ...
        String subtitle = fastSubtitle(para);
        book.setSubtitle(subtitle);
        ...
        if (!Objects.equals(book.getId(), "")) {
            books.add(book);
        }
    }
}


private static String fastSubtitle(String para) {
    if (para.contains("[Subtitle:")) {
        try {
            return para.substring(para.indexOf("[Subtitle:") + 10, para.indexOf("]", para.indexOf("Subtitle:")));
        } catch (StringIndexOutOfBoundsException ex) {
            try {
                return para.substring(para.indexOf("[Language:") + 10, para.indexOf("[") - 1);
            } catch (StringIndexOutOfBoundsException ey) {
                return para.substring(para.indexOf("[Language:") + 10, para.length() - 1);
            }
        }
    }
    return "";
}

I am aware that I could use an if statements in the subtitle method but this does the job.

|improve this answer|||||
\$\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.