Finding vowels, digits, whitespaces and consonants

Question

I am creating a simple app to find vowels, digits, whitespaces and consonants. the basic idea is to practice multi-threading and writing readable code.

import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import javafx.scene.control.TextField;
import javafx.application.Platform;
import javafx.scene.control.Button;
import java.util.stream.IntStream;
import javafx.fxml.Initializable;
import java.util.ResourceBundle;
import javafx.event.ActionEvent;
import java.util.Arrays;
import javafx.fxml.FXML;
import java.util.List;
import java.net.URL;


/**
 *
  * @author Aamir khan
 */
public class FXMLDocumentController implements Initializable {

@FXML
TextField inputField;
@FXML
TextField vowelsField;
@FXML
TextField consField;
@FXML
TextField digitField;
@FXML
TextField spaceField;
@FXML
Button check;
@FXML
Button exit;

private List<Runnable> tasks;


@Override
public void initialize(URL url, ResourceBundle rb) {
    tasks = Arrays.asList(
            () -> findAndUpdateVowels(),
            () -> findAndUpdateCons(),
            () -> findAndUpdateDigits(),
            () -> findAndUpdateWhiteSpace()
    );

    exit.setOnAction(e -> System.exit(0));
}

@FXML
void checkBtnHandle(ActionEvent e) {
    ExecutorService thread = Executors.newFixedThreadPool(tasks.size());
    tasks.forEach(thread::submit);
    thread.shutdown();
}

public void findAndUpdateVowels() {
    long numberOfVowels = find(Find.VOWELS);
    updateField(vowelsField, numberOfVowels);
}

void findAndUpdateWhiteSpace() {
    long numberOfWhiteSpaces = find(Find.WHITE_SPACE);
    updateField(spaceField, numberOfWhiteSpaces);
}

void findAndUpdateCons() {
    long numberOfCons = find(Find.CONSONANTS);
    updateField(consField, numberOfCons);
}

void findAndUpdateDigits() {
    long numberOfDigits = find(Find.DIGITS);
    updateField(digitField, numberOfDigits);

}

public boolean isVowel(int codePoint) {
    //checking for lower case because the input will only in lower case
    return (codePoint == 'a')
            || (codePoint == 'e')
            || (codePoint == 'i')
            || (codePoint == 'o')
            || (codePoint == 'u');

}



private void updateField(TextField field, long val) {
    Platform.runLater(() -> field.setText(String.valueOf(val)));
}


private long find(Find whatToFind) {
    final IntStream chars = inputField.getText().toLowerCase().chars();
    long result = 0L;
    switch (whatToFind) {
        case VOWELS:
            result = chars.filter(this::isVowel).count();
            break;
        case CONSONANTS:
            //Note: we are not preventing any special chars such as œ,Ž,Ÿ
            result =  chars.filter(c -> Character.isLetter(c) && !isVowel(c)).count();
            break;
        case DIGITS:
            result = chars.filter(Character::isDigit).count();
            break;
        case WHITE_SPACE:
            result = chars.filter(Character::isWhitespace).count();
            break;
        default:
            result = 0L;
    }
    return result;
}

enum Find {
    VOWELS,
    WHITE_SPACE,
    DIGITS,
    CONSONANTS;
}
}

Is code readable? How can I optimize it?

Answer 1

Your code contains a lot of duplication. It can be made much shorter by passing a predicate to the find method instead of an enum. When you do that, the whole enum Find will become unused. After that, you should rename the find method to count, so that the name matches what it is actually doing.

Your way of organizing the imports looks unusual and funny. But in the rare case that someone really looks at them (like in this code review) it makes more sense to sort them by topic instead of by length. But if it's your point that these lines are usually not looked at, it's fine.

I would make the code as simple as this:

import javafx.application.Platform;
import javafx.event.ActionEvent;
import javafx.fxml.FXML;
import javafx.fxml.Initializable;
import javafx.scene.control.Button;
import javafx.scene.control.TextField;

import java.net.URL;
import java.util.Arrays;
import java.util.List;
import java.util.ResourceBundle;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.function.IntPredicate;

public class FXMLDocumentController implements Initializable {

    @FXML
    TextField inputField;
    @FXML
    TextField vowelsField;
    @FXML
    TextField consField;
    @FXML
    TextField digitField;
    @FXML
    TextField spaceField;
    @FXML
    Button check;
    @FXML
    Button exit;

    @Override
    public void initialize(URL url, ResourceBundle rb) {
        exit.setOnAction(e -> System.exit(0));
    }

    @FXML
    void checkBtnHandle(ActionEvent e) {
        List<Runnable> tasks = Arrays.asList(
                () -> updateField(vowelsField, this::isVowel),
                () -> updateField(consField, c -> Character.isLetter(c) && !isVowel(c)),
                () -> updateField(digitField, Character::isDigit),
                () -> updateField(spaceField, Character::isWhitespace)
        );

        ExecutorService executor = Executors.newFixedThreadPool(tasks.size());
        tasks.forEach(executor::submit);
        executor.shutdown();
    }

    private void updateField(TextField field, IntPredicate predicate) {
        String lowerCaseText = inputField.getText().toLowerCase();
        long count = lowerCaseText.chars().filter(predicate).count();
        Platform.runLater(() -> field.setText(String.valueOf(count)));
    }


    private boolean isVowel(int codePoint) {
        //checking for lower case because the input will only in lower case
        return codePoint == 'a'
                || codePoint == 'e'
                || codePoint == 'i'
                || codePoint == 'o'
                || codePoint == 'u';
    }
}

I moved the creation of the tasks lists into the checkBtnHandle method since I don't know at which exact instant the initialize method is called, and I wanted to avoid NullPointerExceptions.

Answer 2

The code looks good, very readable, written in a functional style. My only recommendation (beside the comment in the other response) is to separate the concerns i.e. implement the character type counting functionality in a class distinct from UI. At a minimum this should make writing unit tests easier.