I have written a java program to get a series of inputs from the user, step by step with the feature to move to the next question only after getting the valid input from the user like this, for example,
while(true){
while(true){
System.out.println("Enter ID (3 digits)");
int id = sc.nextInt();
if(validateID(id))
break
}
while(true){
System.out.println("Enter Favourite Number (2 digits)");
int fav = sc.nextInt();
if(validateFavNo(fav))
break
}
System.out.println("ID = "+id+" Favouroite No = "+fav);
}
This is obviously not the best approach to do this Hence, I came up with a solution, which I am not sure, whether or not it is the best approach, whic is given below, where inputProgress is a static variable which is initialized to 0 at starting.
while (true) {
if (inputProgress == 0) {
System.out.println("Enter the Product ID");
pID = sc.nextInt();
if (vController.validatePIDFormat(pID))
inputProgress += 1;
else
System.out.println("Invalid input");
continue;
}
if (inputProgress == 1) {
System.out.println("Enter the Product Name");
pName = sc.next();
if (vController.validatePNameFormat(pName))
inputProgress += 1;
else
System.out.println("Invalid input");
continue;
}
if (inputProgress == 2) {
System.out.println("Enter the Product Type");
String pType = sc.next();
pTypeID = -1;
if (pType.contains("commodity")) {
pTypeID = 1;
} else if (pType.contains("equity")) {
pTypeID = 2;
} else if (pType.contains("derivative")) {
pTypeID = 3;
}
if (pTypeID > 0 && pTypeID < 4) {
inputProgress += 1;
} else {
System.out.println("Invalid input");
continue;
}
}
if (inputProgress == 3) {
System.out.println("Enter the Quantity");
pQuantity = sc.nextInt();
if (vController.validatePQuantity(pQuantity)) {
inputProgress += 1;
} else {
System.out.println("Invalid input");
continue;
}
}
Product newProduct = new Product(pID, pName, pQuantity, pTypeID);
inputProgress = 0;
return newProduct;
}
My question is, whether this is a good one or is there a better approach/practice to achieve the same results as desired.
I don't like while(true), there is always a better (and more elegant) solution.
In your case there is a lot of "inelegance", one for all return statement inner the loop, and continue to jump at the top ... no, I don't like it.
To maintain your structure, we should make it more "elengant", like this:
public Product readProduct() {
Scanner sc = new Scanner(System.in);
MockValidator vController = new MockValidator(); // mock
// fields outside loop
int pID = -1;
String pName = null;
int pTypeID = -1;
int pQuantity = -1;
int inputProgress = 0;
while (inputProgress < 4) { // Loop exit condition linked to inputProgress. Exit when read all 4 valid inputs
if (inputProgress == 0) {
System.out.println("Enter the Product ID");
pID = sc.nextInt();
if (vController.validatePIDFormat(pID)) {
inputProgress += 1;
} else {
System.out.println("Invalid input");
}
}
if (inputProgress == 1) {
System.out.println("Enter the Product Name");
pName = sc.next();
if (vController.validatePNameFormat(pName)) {
inputProgress += 1;
} else {
System.out.println("Invalid input");
}
}
if (inputProgress == 2) {
System.out.println("Enter the Product Type");
String pType = sc.next();
pTypeID = -1;
if (pType.contains("commodity")) {
pTypeID = 1;
} else if (pType.contains("equity")) {
pTypeID = 2;
} else if (pType.contains("derivative")) {
pTypeID = 3;
}
if (pTypeID > 0) {
inputProgress += 1;
} else {
System.out.println("Invalid input");
}
}
if (inputProgress == 3) {
System.out.println("Enter the Quantity");
pQuantity = sc.nextInt();
if (vController.validatePQuantity(pQuantity)) {
inputProgress += 1;
} else {
System.out.println("Invalid input");
}
}
}
return new Product(pID, pName, pQuantity, pTypeID);
}
but, wanna be more "elegant", there are many concept repeated in your code, so we can extract some common behavior. Practically, for each parameter, you want to :
Supplier )Function )Predicate )You can write a generic method to do that and use it for every parameter you want !
That's what I'm talking about
private Product readProductSmart() {
Scanner sc = new Scanner(System.in);
MockValidator vController = new MockValidator(); // mock
Map<String, Integer> productTypeMap = Map.of(
"commodity", 1,
"equity", 2,
"derivative", 3
);
int id = readUntilIsValid("Product ID", sc::nextInt, Function.identity(), vController::validatePIDFormat);
String name = readUntilIsValid("Product Name", sc::next, Function.identity(), vController::validatePNameFormat);
int typeId = readUntilIsValid("Product Type", sc::next, productTypeMap::get, Objects::nonNull);
int quantity = readUntilIsValid("Quantity", sc::nextInt, Function.identity(), vController::validatePQuantity);
return new Product(id, name, quantity, typeId);
}
private <S, T> T readUntilIsValid(String name, Supplier<S> supplier, Function<S, T> mapper, Predicate<T> validator) {
T result = null;
boolean valid = false;
while (!valid) { // Repeat until result is valid
System.out.println("Enter the " + name);
S read = supplier.get(); // Read each parameter of a certain type
result = mapper.apply(read); // Convert it in another type (sometimes)
if (!(valid = validator.test(result))) { // Validate it
System.out.println("Invalid input");
}
}
return result;
}
that's solution is pretty "elegant" and flexible (good for maintenance): you can easily add, remove and switch parameter(s).