patternjavaMinor
Finding vowels, digits, whitespaces and consonants
Viewed 0 times
whitespacesvowelsdigitsconsonantsfindingand
Problem
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 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
retu
```
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 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
retu
Solution
Your code contains a lot of duplication. It can be made much shorter by passing a predicate to the
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:
I moved the creation of 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 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.Code Snippets
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';
}
}Context
StackExchange Code Review Q#138252, answer score: 2
Revisions (0)
No revisions yet.