Confuse a text without making it unreadable

Question

Scientists have found that a text can still be read even if only the first and last letter of a word are in their right place. The remaining characters within the word can be arranged randomly.

Suppose you want to send a text whose content is checked for specific words by a filter. Instead of encrypting it, one could also use this algorithm. But actually that's just a fun program.

I would like to know how professionals rate my code. Can you write parts of the source code more readable?

Example Output

Sntecistis hvae fnuod taht a txet can sltil be read eevn if only the fsrit and lsat 
lteter of a wrod are in teihr rghit plcae. The rinnimaeg caartcehrs witihn the wrod 
can be araergnd rdnlomay.

Sppuose you want to sned a text wsohe cnteont is cehcekd for seipcfic wdros by a 
ftelir. Intsead of enintrpcyg it, one cuold aslo use this aorltihgm. But aallucty 
that's jsut a fun prraogm.

I would like to know how pnarloofesiss rtae my code. Can you wtrie ptars of the 
suorce cdoe mroe rabdleae?

Another Example

iormpt jvaa.uitl.Scenanr;
imorpt java.io.Flie;
improt jvaa.io.FdeliRaeer;
import jvaa.io.BfeaeudfeRrder;
ipomrt java.io.IxptiEeOcon;

pilubc cslas Main {
    pilubc siattc viod main(String[] args) {
        SignldetrBuir text = new SeuitdrBginlr();
        if (agrs.lgenth == 0) {
            Senacnr scennar = new Snenacr(Syestm.in);
            text.aeppnd(snncear.niLxtnee());
        } esle if (args[0].conatnis("-help")) {
            Stysem.out.pirlntn("Jsut clal it lkie taht: java Main");
            Seytsm.out.ptnlirn("Or povirde a txt-file: java Main file.txt");
            Sestym.exit(0);
        } esle {
            // read whloe file in Sntirg
            File file = new File(args[0]);
            try (BeafedurfRdeer br = new BerfuReeadedfr(new FadleeReir(flie))) {
                Snritg lnie;
                wihle ((line = br.rLniadee()) != null) {
                    text.apnepd(lnie + "\n");
                }
            } ccath (IOtExeocipn e) {
                e.pacttSTcnrrkiae();
            }
        }

        TeuCxeosfntr tc = new TsCxtofenuer(txet.totnriSg());
        tc.cnufsoe();
        Setsym.out.priltnn(tc.gTdoxseCfneutet());
    }
}

And here is the source code

Main.java

import java.util.Scanner;
import java.io.File;
import java.io.FileReader;
import java.io.BufferedReader;
import java.io.IOException;

public class Main {
    public static void main(String[] args) {
        StringBuilder text = new StringBuilder();
        if (args.length == 0) {
            Scanner scanner = new Scanner(System.in);
            text.append(scanner.nextLine());
        } else if (args[0].contains("-help")) {
            System.out.println("Just call it like that: java Main");
            System.out.println("Or provide a txt-file: java Main file.txt");
            System.exit(0);
        } else {
            // read whole file in String
            File file = new File(args[0]);
            try (BufferedReader br = new BufferedReader(new FileReader(file))) {
                String line;
                while ((line = br.readLine()) != null) {
                    text.append(line + "\n");
                }
            } catch (IOException e) {
                e.printStackTrace();
            }
        }

        TextConfuser tc = new TextConfuser(text.toString());
        tc.confuse();
        System.out.println(tc.getConfusedText());
    }
}

Text Confuser.java

import java.util.List;
import java.util.ArrayList;
import java.util.Random;

public class TextConfuser {
    private String text;
    private List<String> words;
    private StringBuilder confusedText;

    public TextConfuser(String text) {
        this.text = text;
        this.words = new ArrayList<String>();
        this.confusedText = new StringBuilder();
    }

    public void setText(String text) {
        this.text = text;
        this.confusedText = new StringBuilder();
    }

    public String getText() {
        return text;
    }

    public String getConfusedText() {
        return confusedText.toString();
    }

    // make text to a list of words for further processing
    public void parse() {
        // a word can be a word like you know it from natural language
        // special symbols like punctuation characters and control characters are treated as own words
        words = new ArrayList<String>();
        char[] characters = text.toCharArray();
        StringBuilder currentWord = new StringBuilder();
        for (int i = 0; i < characters.length; i++) {
            if (Character.isLetter(characters[i])) {
                currentWord.append(characters[i]);
            } else {
                if (currentWord.length() > 0) {
                    words.add(currentWord.toString());
                    currentWord = new StringBuilder();
                } 
                   words.add(Character.toString(characters[i]));    
            }
        }

        StringBuilder result = new StringBuilder();
        for (String word : words) {
            result.append(word);
        }
    }

    public void confuse() {
        parse();
        for (String word : words) {
            if (word.length() > 3) {
                // get first and last character
                char firstChar = word.charAt(0);
                char lastChar = word.charAt(word.length() - 1);

                // get list of characters between first and last character
                List<Character> chars = new ArrayList<>();
                for (int i = 1; i < word.length() - 1; i++) {
                    chars.add(word.charAt(i));
                }

                // construct confused word
                StringBuilder confusedWord = new StringBuilder();
                confusedWord.append(firstChar);

                while (!chars.isEmpty()) {
                    Random random = new Random();
                    Character randomChar = chars.get(random.nextInt(chars.size()));
                    chars.remove(randomChar);
                    confusedWord.append(randomChar);
                }

                confusedWord.append(lastChar);

                confusedText.append(confusedWord);
            } else {
                confusedText.append(word);
            }
        }
    }
}

Answer 1

The focus of my answer will be on the public API of your TextConfuser class.

API discussion

You chose to make the text, the word list, and the confused text to become fields of this class, so a given TextConfuser can only operate on one text at a time, implying that a user will create a TextConfuser for a given text, and have it compute the confused result. For the next text, he'd create a new TextConfuser instance, and that's absolutely a valid lifecycle pattern.

What doesn't fit into that lifecycle, is the possibility to change the text via the setText() method - you should delete that one. If you want one TextConfuser to be able to convert more that one text, I'd recommend a completely different set of public methods (see further below).

Then, your API is forcing the user to follow a three-step sequence to get results:

    TextConfuser tc = new TextConfuser(input);
    tc.confuse();
    String result = tc.getConfusedText();

This is error-prone: if your user forgets the tc.confuse() step, he'll get an empty result without any notice that he did something wrong.

I'd like to see that reduced to

    TextConfuser tc = new TextConfuser(input);
    String result = tc.getConfusedText();

There are two ways how to achieve that:

• Have the constructor call confuse(), so you immediately have the results available (I can't plainly recommend that, as some developers don't like constructors to do "real work").
• Have the getConfusedText() method check whether the result is already available (and call confuse() if not), and return that.

Then, methods that are only used inside your class should be declared private. This applies to parse() (and confuse() if you follow my recommendation). Having public methods that aren't meant to be called by your user will only confuse him.

An alternative API, allowing for a TextConfuser to be used for multiple texts, even in multiple threads parallel, would be:

public TextConfuser() {...}
public String getConfusedText(String input) {...}

You'd eliminate the fields, and instead pass text, words, and confusedText between the private methods like parse() and confuse():

private List<String> parse(String input) {...}
private String confuse(List<String> words) {...}

Then, your user can do things like

TextConfuser tc = new TextConfuser();
String result1 = tc.getConfusedText("I would like to know how professionals rate my code.");
String result2 = tc.getConfusedText("But actually that's just a fun program.");

Personally, I'd prefer that usage style over the first one.

Coding style

Thumbs up for following the naming conventions, for (mostly) properly indenting your code, for choosing useful variable and method names.

A few improvements are possible:

There's the Collections.shuffle() method you could use in place of your shuffling while loop.

The setText() method doesn't reset the words field, it leaves the TextConfuser in a confusing state where the text and the words don't match.

The snippet

    StringBuilder result = new StringBuilder();
    for (String word : words) {
        result.append(word);
    }

doesn't serve a purpose, as you don't use the result variable anywhere.

Some of your comments should become JavaDoc, especially to describe public methods:

/**
 * make text to a list of words for further processing.
 * A word can be a word like you know it from natural language.
 * Special symbols like punctuation characters and control characters are treated as own words
 */
public void parse() { ... }

Answer 2

I would like to know how professionals rate my code

Sorry to disappoint you :P But maybe some thinks that are in my mind will help you some day. This review is in addition to @Ralf Kleberhoff

---

Maybe a Bug

When I run

public static void main(String[] args) {
    TextConfuser textConfuser = new TextConfuser("Excellent Sir");
    textConfuser.parse();
    textConfuser.confuse();
    System.out.println(textConfuser.getConfusedText());
}

The last word is lost..

Elenxeclt 

Diamond Operator

Instead of specifying the types for the object twice, the diamond operator, <>, can be specified as long as the compiler can infer the types from the context. As such, the diamond operator can be used when instantiating the object

In your code base you have

this.words = new ArrayList<String>()

words = new ArrayList<String>();

If you are useing Java 7+ you can simply write new ArrayList<>().

---

Object Creation

while (!chars.isEmpty()) {
   Random random = new Random();
   // ..
}

This while-loop creates for each element in chars a new instance of Random. Better would be to hoist the initialization above the while loop to initialize it only once.

Random random = new Random();
while (!chars.isEmpty()) {
    // ..
}

---

Comments

Robert C. Martin, who wrote the book "Clean Code" and many more, sad

Don’t Use a Comment When You Can Use a Function or a Variable

You did a good job with your comments and group them to logical units but we can extract these logical units into their own methods

public void confuse() {
    parse();
    for (String word : words) {
        if (word.length() > 3) {
            char firstChar = extractFirstCharacter(word);
            char lastChar = extractLastCharacter(word);
            List<Character> chars = extractCharactersBetweenFirstAndLastCharacter(word);
            StringBuilder confusedWord = constructConfusedWordBy(chars);
            confusedText.append(confusedWord);
        } else {
            confusedText.append(word);
        }
    }
}

---

Primitive Obsession, Feature Envy & Value Object

Primitive Obsession is using primitive data types to represent domain ideas. For example, we use a String to represent a message [...]

Our Primitive Obsession is word. It is from type String and we can easily create a new class Word, which would be a Value Object.

The whole point of objects is that they are a technique to package data with the processes used on that data. A classic [code] smell is a method that seems more interested in a class other than the one it is in.

This quote means in our case that TextConfuser is more busy with manipulating a word that to confuse hole sentence.

public void confuse() {
    parse();
    for (Word word : words) {
        Word confused = word.randomizeReadable();
        confusedText.append(confused.get())    
    }
}

The method randomizeReadable on Word has the hole logic how to manipulate a string.

Answer 3

There are essentially three parts in this problem:

1. Split stream to tokens.
2. Confuscate (is that even a word) a token.
3. Write tokens to a stream.

To maintain the single responsibility principle, these parts should be separated into their own classes. Now all of yourt code is in one class and unit testing it will be fairly difficult.