TL; DR
- вызов одного и того же метода внутри нескольких блоков
if else
является избыточным (до тех пор, пока он не вернется, поэтому требуемый метод все еще может быть вызван) - много раз условие long
if
может быть упрощено, если нет, то лучше переместить условие в дополнительный метод boolean isAThingIWant(..params)
, чтобы сохранить читабельность - , метод "business logi c" не должен вызываться в конструкторе
- создание новых объектов того же класса внутри методов-членов может привести к нежелательным рекурсивным вызовам
- закрытие объекта Scanner с
System.in
InputStream запрещает любые последующие чтения из этого потока (см. this ) - попытка использовать
Double.parseDouble
с любой строкой, не являющейся числом, приведет к NumberFormatException
, даже для инженерной нотации - пример OP - отдельная проблема, но при разработке класса следует использовать инкапсуляцию propper
- обычной практикой является использование стандартов кодирования для конкретного языка (см. this ) * 10 28 *
- OOP подход к решению этой проблемы заключался бы в использовании интерфейса формы и дополнительных классов, реализующих его, что позволило бы использовать преимущества наследования и полиморфизма
Оригинал:
Самый простой Чтобы исправить это, нужно удалить методы Main s = new Main();
снизу и использовать ссылку this
для текущего созданного объекта, как показано ниже:
public void surfaceAreaPrint() throws Exception{
// Main s = new Main();
System.out.println(this.surfaceArea(hP, rP));
System.out.println(this.surfaceArea(rP));
System.out.println(this.surfaceArea(hP, rP, sP));
System.out.println(this.surfaceArea(aP, bP, cP, lP, hP));
}//end surfaceAreaPrint
public void volumePrint() throws Exception{
// Main s = new Main();
System.out.println(this.surfaceArea(hP, rP));
System.out.println(this.surfaceArea(rP));
System.out.println(this.surfaceArea(hP, rP));
System.out.println(this.surfaceArea(bP, lP, hP));
}//end volumePrint
На самом деле, в вашем примере есть несколько проблем и запахов кода.
Другое дело, что вы все еще сравниваете строки с ==
в операторе if
в методе parse
- используйте euqals or equalsIgnoreCase
.
Если ваша попытка была действительно новой Если пользователь вводит указанные выше методы, вы не можете закрыть сканер, потому что закрывая его с помощью System.in
, вы закрываете InputStream за ним, поэтому он больше ничего не принимает. Я думаю, что удаление sc.close()
может привести к различным исключениям и проблемам (например, программа не будет работать так, как вы этого хотите).
Каждая инструкция в любом else if
используется одним и тем же способом. Поэтому целое if else if
является избыточным.
Edit:
Только что проверил, что удаление sc.close()
и сохранение остатка, как в вашем примере, приведет к бесконечному получению пользовательского ввода из-за создания новый объект Main в surfaceAreaPrint
и вход вызывается из конструктора Main, поэтому он становится рекурсивным.
Для использования перегруженного метода вы можете go просто так:
//cylinder
if(a.equalsIgnoreCase ("none") && b.equalsIg... rest of your if) {
System.out.println(this.surfaceArea(hP, rP));
}
Но учтите, что когда вы просто нажимаете ввод при вводе значений и читаете их с помощью sc.nextLine()
, назначенное значение не останется как none
, но станет пустой строкой, такой как ""
, которая будет выбрасывать NumberFormatException
позже в любом из Double.parseDouble
.
Edit2:
Теперь я вижу, что вы хотите строго ввести "none", когда вы не будете использовать эту конкретную переменную, поэтому избегайте NumerFormatException
Вы должны проанализировать значения, чтобы удвоить их после проверки, не является ли строка «none».
Вы можете упростить операторы if, изменив проверку, если они равны «none», чтобы проверить, не имеют ли они:
* 107 5 *
Для расчета поверхности / объема реальных геометрических объектов можно предположить, что значения, определяющие их размер, должны составлять >=0
, поэтому для более простого анализа можно использовать такой метод:
public double inputParser(String s) {
try {
return Double.parseDouble(s);
} catch (NumberFormatException e) {
return -1;
}
}
//and used instead of straight `Double.parseDouble`:
aP = inputParser(a);
bP = inputParser(b);
//which simplifies the `if` even more to:
if(hP != -1 && rP != -1) { //cylinder
System.out.println(this.surfaceArea(hP, rP));
}
Edit3:
После нескольких минут обсуждения и обнаружения некоторого другого fl aws в коде OP я принял решение:
import java.util.*;
class Main {
String a, b, c, h, r, s, l;
String round;
double aP; double bP; double cP; double hP; double rP; double sP; double lP;
public static void main(String[] args) {
Main m = new Main();
m.input();
m.parse();
m.calculate();
}
public void input() {
Scanner sc = new Scanner(System.in);
System.out.print("");
System.out.print("Input your shape’s “a” value: ");
a = sc.nextLine();
System.out.print("Input your shape’s “b” value: ");
b = sc.nextLine();
System.out.print("Input your shape’s “c” value: ");
c = sc.nextLine();
System.out.print("Input your shape’s “h” value: ");
h = sc.nextLine();
System.out.print("Input your shape’s “r” value: ");
r = sc.nextLine();
System.out.print("Input your shape’s “s” value: ");
s = sc.nextLine();
System.out.print("Input your shape’s “l” value: ");
l = sc.nextLine();
System.out.print("Would you like your answer rounded?: ");
round = sc.nextLine();
System.out.println("");
sc.close();
}
public void parse() {
System.out.println(a);
aP = inputParser(a);
bP = inputParser(b);
cP = inputParser(c);
hP = inputParser(h);
rP = inputParser(r);
sP = inputParser(s);
lP = inputParser(l);
}
public void calculate() {
if(hP != -1 && rP != -1) {
System.out.println("Cylinder");
System.out.println(this.rounding(this.surfaceArea(hP, rP)));
System.out.println(this.rounding(this.volume(hP, rP)));
}
if (rP != -1){
System.out.println("Sphere");
System.out.println(this.surfaceArea(rP));
System.out.println(this.volume(rP));
}
if (hP != -1 && rP != -1 && sP != -1){
System.out.println("Cone");
System.out.println(this.surfaceArea(hP, rP, sP));
System.out.println(this.volume(hP, rP, sP));
}
if (aP != -1 && bP != -1 && cP != -1 && lP != -1 && hP != -1){
System.out.println("Triangular prism");
System.out.println(this.surfaceArea(aP, bP, cP, lP, hP));
System.out.println(this.volume(aP, bP, cP, lP, hP));
}
}
public double rounding(double value) {
if ("yes".equalsIgnoreCase(round)) {
return Math.round(value);
}
return value;
}
public double inputParser(String s) {
try {
return Double.parseDouble(s);
} catch (NumberFormatException e) {
return -1;
}
}
//surface area for cylinder
public double surfaceArea(double hP, double rP){
return (2.0 * Math.PI * Math.pow(rP, 2.0)) + (2.0 * Math.PI * rP * hP);
}//end surfaceArea
//surface area for sphere
public double surfaceArea(double rP){
return (4.0 * Math.PI * Math.pow(rP, 2.0));
}//end surfaceArea
//surface area for cone
public double surfaceArea(double hP, double rP, double sP){
return (Math.PI * Math.pow(rP, 2.0)) + (Math.PI * rP * sP);
}//end surfaceArea
//surface area for traingular prism
public double surfaceArea(double aP, double bP, double cP, double lP, double hp){
return (bP * lP) + (aP * hP) + (bP * hP) + (cP* hP);
}//end surfaceArea
//volume for cylinder
public double volume(double hP, double rP){
return (Math.PI * Math.pow(rP, 2.0) * hP);
}//end volume
//volume for sphere
public double volume(double rP){
return ( 4.0 / 3.0 ) * Math.PI * Math.pow(rP, 3.0);
}//end volume
//volume for cone
public double volume(double hP, double rP, double sP){
return (1.0 / 3.0 ) * Math.PI * Math.pow(rP, 2.0) * hP;
}//end volume
//volume for traingular prism
public double volume(double aP, double bP, double cP, double lP, double hp){
return (1.0 / 2.0 ) * bP * lP * hP;
}//end volume
}//end Main
Все еще не самый лучший, с инкапсуляцией, именованием переменных и некоторыми другие вопросы, но хорошо решает проблему.