4
\$\begingroup\$

I need to generate random text for tests. I do that using alphanumeric characters and some special characters as well.

For some special cases, I cannot generate the random text with tags. I have the concept of a tag as text surrounded in curly braces (e.g. {name}).

I have written a simple code to clean up the tags (e.g. removing one of the curly braces) and I'd like some feedback if looks ok for Scala.

val allCharacters = "qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJKLZXCVBNM1234567890!@#%*()_+-=[]{x}|?.;:`"

def removeTags(text: String): String = {

    // A tag is any text between {}, like `{name}`
    def containsTag(text: String): Boolean = {
        val noTagsRegex: Pattern = Pattern.compile("^((?!(\\{.*\\})).)*$")
        !noTagsRegex.matcher(text).matches()
    }

    var newText = text
    if (containsTag(newText)) {
        val randomPosition = Random.nextInt(allCharacters.length - 1)
        newText = text.replace("}", allCharacters(randomPosition).toString)
    }
    newText
}

Below an idea of results running this method:

  1. Text without tags

    • input: "dlfkhfks sldfjhsldf qiweo 93450934 %&%&%&^%"
    • output: "dlfkhfks sldfjhsldf qiweo 93450934 %&%&%&^%" (exactly the same)
  2. Text with 1 tag

    • input: "dlfkhfks s{ldf}jhsldf qiweo 93450934 %&%&%&^%"
    • output: "dlfkhfks s4ldf}jhsldf qiweo 93450934 %&%&%&^%" (the { was replaced)
  3. Text with more than 1 tag

    • input: "dlfkhfks s{ldf}jhsldf {{@}} qiweo 93450934 %&%&%&^%"
    • output: "dlfkhfks s4ldf}jhsldf 44@}} qiweo 93450934 %&%&%&^%"

There is room for improvements, like not doing replace to all occurrences by the same character, but this is something for another moment.

\$\endgroup\$
  • \$\begingroup\$ can you look at your question one more again and maybe add some more info like input texts or another? I see some problems in what you write in question and what in your code: regular expression do not find curly braces for simple example like sometext {name} sometext (look at: regex101.com/r/2PR2kC/1) and in next you replace "}" for random symbol from allCharacters where you also has curly braces. \$\endgroup\$ – Gudsaf Feb 28 at 20:58
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Mast Mar 1 at 18:02
  • \$\begingroup\$ @Gudsaf No. A new question should be posted. \$\endgroup\$ – Mast Mar 1 at 18:03
  • \$\begingroup\$ @Mast: Thank you for clarifying that. Won’t happen again. \$\endgroup\$ – Tom Mar 1 at 20:46
4
\$\begingroup\$

Beautify your regex

Say No to Pattern.compile and instead of

 Pattern.compile("^((?!(\\{.*\\})).)*$")

use scalish way:

 "^((?!(\\{.*\\})).)*$".r

Say No to \\ in regex and instead of

"^((?!(\\{.*\\})).)*$"

use """ way:

"""^((?!(\{.*\})).)*$"""

Return values without val/var

You can simply write your containsTag without val and result of last line will return from function - it will return your's matches:

def containsTag(text: String): Boolean = {
  ! """^((?!(\{.*\})).)*$""".r.matches(text)
}

Choose your destiny

Sometimes it is good to use if/else but more readable match/case:

containsTag(text) match {
  case true =>
    val randomPosition = Random.nextInt(allCharacters.length - 1)
    text.replace("}", allCharacters(randomPosition).toString)
  case _ =>
    text
}

Feel the power

Collect it all together:

  val allCharacters = "qwertyuiopasdfghjklzxcvbnmQWERTY" +
    "UIOPASDFGHJKLZXCVBNM1234567890!@#%*()_+-=[]{x}|?.;:`"

  def removeTags(text: String): String = {

    def containsTag(text: String): Boolean = {
      ! """^((?!(\{.*\})).)*$""".r.matches(text)
    }

    containsTag(text) match {
      case true =>
        val randomPosition = Random.nextInt(allCharacters.length - 1)
        text.replace("}", allCharacters(randomPosition).toString)
      case _ =>
        text
    }
  }

  val res = removeTags("my friendly {name} is $userNickname$")

For random chars

According to answer in Replacing characters in a String in Scala you can use construction without while loop but with map + lambda like in code below:

  val allCharacters = "+=-?"

  def removeTags(text: String): String = {

    def containsTag(text: String): Boolean = {
      ! """^((?!(\{.*\})).)*$""".r.matches(text)
      true
    }

    containsTag(text) match {
      case true =>
        text.map(
          (c) => c match {
            case '{' =>
              val randomPosition = Random.nextInt(allCharacters.length - 1)
              allCharacters(randomPosition)
            case c => c // for another char which is not == {
          }
        )
      case _ =>
        text
    }
  }

Then it will be good to divide functionality and create new Char => Char transformer function from anonymous (c) => c match {..} which we lay in text.map(...):

//...
def replaceRandom(c: Char): Char = {
  c match {
    case '{' =>
      val randomPosition = Random.nextInt(allCharacters.length - 1)
      allCharacters(randomPosition)
    case c => c
  }
}

containsTag(text) match {
  case true => text.map((c) => {replaceRandom(c)})
  case _ => text
}
//...

Simplify code in two parts.

Part 1: simplify match/case in replaceRandom(c: Char): Char :

def replaceRandom(c: Char): Char = {
  c match {
    case '{' =>
      val randomPosition = Random.nextInt(allCharacters.length - 1)
      allCharacters(randomPosition)
    case c => c
  }
}

// remove unneeded "{" and "}"
def replaceRandom2(c: Char): Char = c match {
  case '{' =>
    val randomPosition = Random.nextInt(allCharacters.length - 1)
    allCharacters(randomPosition) 
  case c => c
}

Then part 2: simplify map((c) => {replaceRandom(c)}):

containsTag(text) match {
  case true => text.map((c) => {replaceRandom(c)})
  case _ => text
}

//remove unneeded "{" and "}"
containsTag(text) match {
  case true => text.map((c) => replaceRandom(c))
  case _ => text
}

//remove unneeded "(c)" to wildcard "_"
containsTag(text) match {
  case true => text.map(replaceRandom(_))
  case _ => text
}

//remove unneeded wildcard "_"
containsTag(text) match {
  case true => text.map(replaceRandom)
  case _ => text
}

Feel the power again

So for your task i will chose that variant: all functions are divided, logic placed is simple if/else and now good readable (as you see now it good readable also with if/else as i wrote to you above). After, when logic will grow you can use instead if/else match/case.

  def removeTags(text: String): String = {

    def containsTag(text: String): Boolean = {
      ! """^((?!(\{.*\})).)*$""".r.matches(text)
      true
    }

    def replaceRandom(c: Char): Char = c match {
      case '{' =>
        val randomPosition = Random.nextInt(allCharacters.length - 1)
        allCharacters(randomPosition)
      case c => c
    }

    if (containsTag(text))
      text.map(replaceRandom)
    else text

  }

  val allCharacters = "+=-?"
  val e = "my fr{{{iend{{{ly {{name} is $userNickname$"
  println(removeTags(e))

P.S. or place to think

  1. Maybe your regex not working? Check it at: Regex101
  2. Why you need to replace } symbol with another random from allCharacters which can be also }?
  3. The bad, the good and the !: maybe try to rewrite your !noTagsRegex to noTagsRegex? It can be more readable for another people.

Also i want to see additions to my review too, that code is too verbose, so welcome!

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ @Gussaf: Thanks a lot. This is a valuable feedback for me. I’m wondering if I replace the match by a while loop, it can try to replace the curly braces using the allCharacters variable until something that doesn’t match a tag. \$\endgroup\$ – Tom Feb 29 at 13:28
  • \$\begingroup\$ @Tom: why you need to replace match/case to while loop? I think that it is necessary to describe the task. You want to create loop which will replace curly braces over number of texts like "take text1 and replace braces" and then "take text2 and replace braces" and then "take text3..." and so on? \$\endgroup\$ – Gudsaf Feb 29 at 14:30
  • 1
    \$\begingroup\$ @Gussaf: I've added more information in the question. Sorry if it wasn't clear before. The while loop would be useful if by picking a random character from allCharacters results in { again. \$\endgroup\$ – Tom Mar 1 at 6:36
  • 1
    \$\begingroup\$ @Tom ok, update answer with that information - it really help to solve problem) \$\endgroup\$ – Gudsaf Mar 1 at 12:25

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.