Of course it's nested. Nesting has nothing to do with the way you format the code.
if (...)
{
// some code
}
else if (...)
{
// some code
}
else
{
// some code
}
is exactly equivalent to
if (...)
{
// some code
}
else
{
if (...)
{
// some code
}
else
{
// some code
}
}
There's no 'else if' keyword in Java, the effect is achieved purely by a formatting convention which stops long chains of nested elses from drifting right across the screen.
In case anybody needs persuading, here is the specification for an if statement: Java Language Specification section 14.9
Answer from JeremyP on Stack Overflowif statement - Nested if - else in java - Stack Overflow
In Java, is this considered an example of a "nested IF statement"? - Stack Overflow
java - Avoiding nested if statements - Stack Overflow
if statement - Nested if-else vs if-else? - Stack Overflow
Videos
Of course it's nested. Nesting has nothing to do with the way you format the code.
if (...)
{
// some code
}
else if (...)
{
// some code
}
else
{
// some code
}
is exactly equivalent to
if (...)
{
// some code
}
else
{
if (...)
{
// some code
}
else
{
// some code
}
}
There's no 'else if' keyword in Java, the effect is achieved purely by a formatting convention which stops long chains of nested elses from drifting right across the screen.
In case anybody needs persuading, here is the specification for an if statement: Java Language Specification section 14.9
This is an if statement:
if (condition1) {
// ...
}
This is a nested if statement:
if (condition1) {
// ...
if (condition2) {
// ...
}
// ...
}
This is an if-else statement:
if (condition1) {
// ...
} else {
// ...
}
This is a chained if-else statement:
if (condition1) {
// ...
} else if (condition2) {
// ...
}
You can indeed reduce the level of nesting significantly. However I leave it up to you whether you like it or not.
Your code currently has three levels of nesting, that's relatively okay. Use comments to make your code more readable instead. However you may adapt some of the following tricks.
Is still unassigned
String returnMessage = null;
// Something is empty
if (cipherFileRead.isEmpty() || freqFileRead.isEmpty()) {
returnMessage = "Both files must be loaded and contain text. Decryption unsuccessful.";
}
// No description type given
if (returnMessage == null && freqChooser.getSelection() == null) {
returnMessage = "Please select decryption type. Decryption unsuccessful";
}
// Decrypt
if (returnMessage == null) {
// Select technique
String decryptedText = null;
if (nearestFreq.isSelected()) {
decryptedText = decrypter.nearestFreq(cipherFileRead, freqFileRead);
} else if (rankingFreq.isSelected()) {
decryptedText = decrypter.byRanking(cipherFileRead, freqFileRead);
}
// Write decrypted text
file.writeToFile(decryptedText, "output.txt");
returnMessage = "Successfully decrypted to output.txt";
}
// Show return message
JOptionPane.showMessageDialog(null, returnMessage);
So the trick was to check whether returnMessage is still unassigned.
The disadvantage of this style is that it slows the program a bit because it makes it harder for the processor to guess the conditions correct for pipelining. This is called branch prediction and all processors do that.
But if it enhances readability, for example by reducing a really deep nesting, you may use it.
Return from method
However if this is a method I would definitely give the advice to use such a style, but then by using return like:
public String decryptFile() {
// Something is empty
if (cipherFileRead.isEmpty() || freqFileRead.isEmpty()) {
return "Both files must be loaded and contain text. Decryption unsuccessful.";
}
// No description type given
if (freqChooser.getSelection() == null) {
return "Please select decryption type. Decryption unsuccessful";
}
// Decrypt
// Select technique to use
String decryptedText = null;
if (nearestFreq.isSelected()) {
decryptedText = decrypter.nearestFreq(cipherFileRead, freqFileRead);
} else if (rankingFreq.isSelected()) {
decryptedText = decrypter.byRanking(cipherFileRead, freqFileRead);
} else {
// Out of techniques?
return "No decryption technique can be applied.";
// Or if this must not happen due to context, use:
// throw new AssertionError();
}
// Write decrypted text
file.writeToFile(decryptedText, "output.txt");
return "Successfully decrypted to output.txt";
}
And the caller will then do it like:
// Show return message
JOptionPane.showMessageDialog(null, decryptFile());
In this scenario, next to reducing the nesting, you also reduce the amount of time you spend in the code itself.
You should definitely pack this inside a method. It should not be the decision of the decryption where and how to display the return message. Instead return the message and let the caller decide how to use it.
Note that in practice you probably want to add some arguments to the method and so on. Also you may want to use exceptions instead of plain String messages, like IllegalArgumentException.
I'll answer by using the principles of one of my favorite book called "Clean Code":
A function should do only one thing, if you have nested if like this your function is certainly doing more than one thing.
So that means, each if/else if/else block should then be in its own function. This solved the nested if problem.
And the extracted function should be given a descriptive name of what it does. This is even better then commenting it because comment can go out of date, or even irrelevant, and thus become misleading.
So if I were try to refactor your function with the above principles, I may come up with something like this:
public void displayDecryptResult() {
String decryptResult = decrypt();
JOptionPane.showMessageDialog(null, decryptResult);
}
private String decrypt() {
String decryptResult;
if(!cipherFileRead.isEmpty() && !freqFileRead.isEmpty() ){
decryptResult = decryptForValidInputFiles();
} else{
decryptResult = "Both files must be loaded and contain text. Decryption unsuccesful.";
}
return decryptResult;
}
private String decryptForValidInputFiles() {
if (freqChooser.getSelection() != null){
message = decryptForValidInputFilesAndDecryptionType();
} else {
message = "Please select decryption type. Decryption unsuccesful";}
}
return message;
}
private String decryptForValidInputFilesAndDecryptionType() {
if(nearestFreq.isSelected()){
file.writeToFile(decrypter.nearestFreq(cipherFileRead, freqFileRead), "output.txt");;
}
else if (rankingFreq.isSelected()){
file.writeToFile(decrypter.byRanking(cipherFileRead, freqFileRead), "output.txt");;
}
return "Succesfully decrypted to output.txt";
}
Some may find the function name is too descriptive that it becomes laughable. But after a long time you've written the code, and you have to debug it, you would thank you had given those descriptive name, so you can easily skim through the pseudocode like names to get idea of what it's trying to do.
Nested if/else means a heirachy so..
if($something == true){
if($something_else == true){
} else {
}
} else { // $something == false
if($something_else == true){
} else {
}
}
A nested if statement is essentially a series of if statements within each other like so...
if (<condition>)
{
if (<condition>)
{
...
} else {
...
}
} else {
...
}
If-else if statements make using multiple conditions cleaner and easier to read, all being on the same level, like so:
if (<condition 1>)
{
...
}
else if (<condition 2>)
{
...
}
else
{
...
}
I had my first code-review the other day. My boss explains to me that having 4 if statements, each returning out of the method is a much clearer, more readable, smarter way to program.
Wait what? Is this really the case? I know this may be a stylistic preference, but is it not considered bad practice? I was taught that returning nothing in a void method is bad and good logic usually looks like a well-thought out cascading if else.
What are the pros and cons of these two different styles?
edit: examples for clarity. sorry for the formatting i did my best
public void example1(String name){
if(name.equals(""))
return;
if(name==null)
return;
if(name.contains("something"){
out.println("Your name is not okay.");
return;
}
this.name=name;
}
public void example2(String name){
if(name==null||name.equals("")){
return;
}
else if(name.contains("something")){
out.println("your name is not okay")
}
else
this.name=name;
}My boss would prefer example 1, and i would code something like example 2 minus the empty return