Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

The major problem I can see is this piece of code :

 try {
 if (num1.getText().length() == 0 || num2.getText().length() == 0) {
 throw new Exception();
 }
 }
 catch (Exception ex){
 JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
 }

If you throw inside a try-catch, you will not exit the function. Since your condition is num1.getText().length() == 0 when you try to do Double.parseDouble(num1.getTest() well your string is empty so this throw an exception.

You should not use throw and catch to manage an error, not in this way. You should first remove the try-catch and work with a simple if :

if (num1.getText().isEmpty() || num2.getText().isEmpty()) {
 JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
 throw new Exception();
}

I would not suggested that since now you add a complexity to your code.

  • The method now need to declare throws Exception (which is not possible for actionPerformed()), now the caller need to manage this possible outcome.

  • Exception would be too generic.

  • I guess that this case would happen a lot,so throwing an Exception could become heavy for the program.

The real solution is to only return from the method. You've already warned the user that he need to do something (you could be more clear) and you don't want to execute the rest of method.


There is some general notion that are lacking in your code, but I won't cover it all, since I mention it in others answer :

  • There is naming convetions in Java, b_add does not follow it, it should be bAdd (but it could simply be add)

  • You should not extend JFrame, but rather just have a private JFrame, you're not adding any functionnality to JFrame you're only using it. (Composition over inheritance) answer that cover a bit that part answer that cover a bit that part

The major problem I can see is this piece of code :

 try {
 if (num1.getText().length() == 0 || num2.getText().length() == 0) {
 throw new Exception();
 }
 }
 catch (Exception ex){
 JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
 }

If you throw inside a try-catch, you will not exit the function. Since your condition is num1.getText().length() == 0 when you try to do Double.parseDouble(num1.getTest() well your string is empty so this throw an exception.

You should not use throw and catch to manage an error, not in this way. You should first remove the try-catch and work with a simple if :

if (num1.getText().isEmpty() || num2.getText().isEmpty()) {
 JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
 throw new Exception();
}

I would not suggested that since now you add a complexity to your code.

  • The method now need to declare throws Exception (which is not possible for actionPerformed()), now the caller need to manage this possible outcome.

  • Exception would be too generic.

  • I guess that this case would happen a lot,so throwing an Exception could become heavy for the program.

The real solution is to only return from the method. You've already warned the user that he need to do something (you could be more clear) and you don't want to execute the rest of method.


There is some general notion that are lacking in your code, but I won't cover it all, since I mention it in others answer :

  • There is naming convetions in Java, b_add does not follow it, it should be bAdd (but it could simply be add)

  • You should not extend JFrame, but rather just have a private JFrame, you're not adding any functionnality to JFrame you're only using it. (Composition over inheritance) answer that cover a bit that part

The major problem I can see is this piece of code :

 try {
 if (num1.getText().length() == 0 || num2.getText().length() == 0) {
 throw new Exception();
 }
 }
 catch (Exception ex){
 JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
 }

If you throw inside a try-catch, you will not exit the function. Since your condition is num1.getText().length() == 0 when you try to do Double.parseDouble(num1.getTest() well your string is empty so this throw an exception.

You should not use throw and catch to manage an error, not in this way. You should first remove the try-catch and work with a simple if :

if (num1.getText().isEmpty() || num2.getText().isEmpty()) {
 JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
 throw new Exception();
}

I would not suggested that since now you add a complexity to your code.

  • The method now need to declare throws Exception (which is not possible for actionPerformed()), now the caller need to manage this possible outcome.

  • Exception would be too generic.

  • I guess that this case would happen a lot,so throwing an Exception could become heavy for the program.

The real solution is to only return from the method. You've already warned the user that he need to do something (you could be more clear) and you don't want to execute the rest of method.


There is some general notion that are lacking in your code, but I won't cover it all, since I mention it in others answer :

  • There is naming convetions in Java, b_add does not follow it, it should be bAdd (but it could simply be add)

  • You should not extend JFrame, but rather just have a private JFrame, you're not adding any functionnality to JFrame you're only using it. (Composition over inheritance) answer that cover a bit that part

added 537 characters in body
Source Link
Marc-Andre
  • 6.8k
  • 5
  • 39
  • 65

The major problem I can see is this piece of code :

 try {
 if (num1.getText().length() == 0 || num2.getText().length() == 0) {
 throw new Exception();
 }
 }
 catch (Exception ex){
 JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
 }

If you throw inside a try-catch, you will not exit the function. Since your condition is num1.getText().length() == 0 when you try to do Double.parseDouble(num1.getTest() well your string is empty so this throw an exception.

You should not use throw and catch to manage an error, not in this way. You should first remove the try-catch and work with a simple if :

if (num1.getText().isEmpty() || num2.getText().isEmpty()) {
 JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
 throw new Exception();
}

I would not suggested that since now you add a complexity to your code.

  • The method now need to declare throws Exception (which is not possible for actionPerformed()), now the caller need to manage this possible outcome.

  • Exception would be too generic.

  • I guess that this case would happen a lot,so throwing an Exception could become heavy for the program.

The real solution is to only return from the method. You've already warned the user that he need to do something (you could be more clear) and you don't want to execute the rest of method.


There is some general notion that are lacking in your code, but I won't cover it all, since I mention it in others answer :

  • There is naming convetions in Java, b_add does not follow it, it should be bAdd (but it could simply be add)

  • You should not extend JFrame, but rather just have a private JFrame, you're not adding any functionnality to JFrame you're only using it. (Composition over inheritance) answer that cover a bit that part

The major problem I can see is this piece of code :

 try {
 if (num1.getText().length() == 0 || num2.getText().length() == 0) {
 throw new Exception();
 }
 }
 catch (Exception ex){
 JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
 }

If you throw inside a try-catch, you will not exit the function. Since your condition is num1.getText().length() == 0 when you try to do Double.parseDouble(num1.getTest() well your string is empty so this throw an exception.

You should not use throw and catch to manage an error, not in this way. You should first remove the try-catch and work with a simple if :

if (num1.getText().isEmpty() || num2.getText().isEmpty()) {
 JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
 throw new Exception();
}

I would not suggested that since now you add a complexity to your code.

  • The method now need to declare throws Exception (which is not possible for actionPerformed()), now the caller need to manage this possible outcome.

  • Exception would be too generic.

  • I guess that this case would happen a lot,so throwing an Exception could become heavy for the program.

The real solution is to only return from the method. You've already warned the user that he need to do something (you could be more clear) and you don't want to execute the rest of method.

The major problem I can see is this piece of code :

 try {
 if (num1.getText().length() == 0 || num2.getText().length() == 0) {
 throw new Exception();
 }
 }
 catch (Exception ex){
 JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
 }

If you throw inside a try-catch, you will not exit the function. Since your condition is num1.getText().length() == 0 when you try to do Double.parseDouble(num1.getTest() well your string is empty so this throw an exception.

You should not use throw and catch to manage an error, not in this way. You should first remove the try-catch and work with a simple if :

if (num1.getText().isEmpty() || num2.getText().isEmpty()) {
 JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
 throw new Exception();
}

I would not suggested that since now you add a complexity to your code.

  • The method now need to declare throws Exception (which is not possible for actionPerformed()), now the caller need to manage this possible outcome.

  • Exception would be too generic.

  • I guess that this case would happen a lot,so throwing an Exception could become heavy for the program.

The real solution is to only return from the method. You've already warned the user that he need to do something (you could be more clear) and you don't want to execute the rest of method.


There is some general notion that are lacking in your code, but I won't cover it all, since I mention it in others answer :

  • There is naming convetions in Java, b_add does not follow it, it should be bAdd (but it could simply be add)

  • You should not extend JFrame, but rather just have a private JFrame, you're not adding any functionnality to JFrame you're only using it. (Composition over inheritance) answer that cover a bit that part

Source Link
Marc-Andre
  • 6.8k
  • 5
  • 39
  • 65

The major problem I can see is this piece of code :

 try {
 if (num1.getText().length() == 0 || num2.getText().length() == 0) {
 throw new Exception();
 }
 }
 catch (Exception ex){
 JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
 }

If you throw inside a try-catch, you will not exit the function. Since your condition is num1.getText().length() == 0 when you try to do Double.parseDouble(num1.getTest() well your string is empty so this throw an exception.

You should not use throw and catch to manage an error, not in this way. You should first remove the try-catch and work with a simple if :

if (num1.getText().isEmpty() || num2.getText().isEmpty()) {
 JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
 throw new Exception();
}

I would not suggested that since now you add a complexity to your code.

  • The method now need to declare throws Exception (which is not possible for actionPerformed()), now the caller need to manage this possible outcome.

  • Exception would be too generic.

  • I guess that this case would happen a lot,so throwing an Exception could become heavy for the program.

The real solution is to only return from the method. You've already warned the user that he need to do something (you could be more clear) and you don't want to execute the rest of method.

lang-java

AltStyle によって変換されたページ (->オリジナル) /