Это мой первый вопрос, и я хотел бы получить обратную связь. Программа работает, но я не уверен, что классы, которые я использовал, правильные.
Во-первых, класс для расчета индекса массы тела из двойного (кг) и целого числа (рост в см):
//Data taken from https://codereview.stackexchange.com/questions/216933/java-swing-exercise-bmi-calculator
//NavigableMap idea taken from https://codereview.stackexchange.com/questions/142708/school-grading-string-calculator/142752#142752
import java.util.Map;
import java.util.NavigableMap;
import java.util.TreeMap;
public class PersonHealthData {
private final double weight; //Kg.
private final int height; //cm.
private static final NavigableMap<Double, String> grades = new TreeMap<>();
static {
grades.put(0.0, "Insuficient Weight");
grades.put(18.5, "Normal Wight");
grades.put(24.9, "Overweight Grade 1");
grades.put(26.9, "Overweight Grade 2: Probesity");
grades.put(29.9, "Obesity Grade 1");
grades.put(34.9, "Obesity Grade 2");
grades.put(39.9, "Obesity Grade 3: Morbid");
grades.put(49.9, "Obesity Grade 4: Extreme");
}
public PersonHealthData(double weight, int height) {
this.weight = weight;
this.height = height;
}
public double getWeight() {
return weight;
}
public int getHeight() {
return height;
}
public double getBMI() {
double heightMeters = height / 100.0;
return weight / (double) (heightMeters * heightMeters);
}
public String getGrade() {
Map.Entry<Double, String> gradeEntry = grades.floorEntry(getBMI());
if (gradeEntry == null) {
return "Incorrect Data";
}
return gradeEntry.getValue();
}
}
Второй класс — это основная программа (создает интерфейс и действует как контроллер, я думаю)
import javax.swing.*;
import java.awt.*;
public class MainProgram {
private final JFrame window;
private final EntryPanel dataEntry;
private final ResultPanel results;
MainProgram() {
window = new JFrame("BMI CAlculator");
window.setLayout(new GridLayout(0, 1));
dataEntry = new EntryPanel(this);
window.add(dataEntry.getPanel());
results = new ResultPanel();
window.add(results.getPanel());
window.pack();
window.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
window.setLocationRelativeTo(null);
}
public void show() {
window.setVisible(true);
}
public static void main(String[] args) {
MainProgram mainProgram;
mainProgram = new MainProgram();
mainProgram.show();
}
public void action() {
double weight;
try {
weight = Double.parseDouble(dataEntry.getStringWeight());
} catch (NumberFormatException e) {
results.error("Weight input is not a number");
return;
}
int height;
try {
height = Integer.parseInt(dataEntry.getStringHeight());
} catch (NumberFormatException e) {
results.error("Height input is not a number");
return;
}
PersonHealthData data = new PersonHealthData(weight, height);
results.setResults(String.format("Your BMI is: %.2f", data.getBMI()), data.getGrade());
}
}
И два класса для практики компонентов и слушателей Swing. Первый — это панель ввода:
import javax.swing.*;
import javax.swing.border.EmptyBorder;
import java.awt.*;
import java.awt.event.KeyAdapter;
import java.awt.event.KeyEvent;
public class EntryPanel {
private final JPanel panel;
private final JPanel panelSliders;
private final JPanel panelInput;
private final JTextField textWeight;
private final JTextField textHeight;
private final MainProgram mainProgram;
private final JSlider sliderWeight;
private final JSlider sliderHeight;
EntryPanel(MainProgram mainProgram) {
this.mainProgram = mainProgram;
panel = new JPanel(new GridLayout(0, 1));
panelSliders = new JPanel(new GridLayout(0, 2));
sliderWeight = new JSlider();
sliderWeight.setMinimum(10);
sliderWeight.setMaximum(200);
panelSliders.add(sliderWeight);
panelSliders.add(new JLabel("Weight"));
sliderHeight = new JSlider();
sliderHeight.setMinimum(10);
sliderHeight.setMaximum(270);
panelSliders.add(sliderHeight);
panelSliders.add(new JLabel("Height"));
panelSliders.setBorder(new EmptyBorder(10, 10, 10, 10));
panel.add(panelSliders);
panelInput = new JPanel(new GridLayout(0, 3, 10, 10));
panelInput.setBorder(new EmptyBorder(10, 10, 10, 10));
panelInput.add(new JLabel("Weight"));
textWeight = new JTextField();
textWeight.addKeyListener(new KeyAdapter() {
@Override
public void keyReleased(KeyEvent e) {
super.keyReleased(e);
mainProgram.action();
}
});
textWeight.setHorizontalAlignment(SwingConstants.RIGHT);
panelInput.add(textWeight);
panelInput.add(new JLabel("kg."));
panelInput.add(new JLabel("Height"));
textHeight = new JTextField();
textHeight.setHorizontalAlignment(SwingConstants.RIGHT);
textHeight.addKeyListener(new KeyAdapter() {
@Override
public void keyReleased(KeyEvent e) {
super.keyReleased(e);
mainProgram.action();
}
});
panelInput.add(textHeight);
panelInput.add(new JLabel("cm."));
sliderWeight.addChangeListener(e -> {
textWeight.setText("" + sliderWeight.getValue());
mainProgram.action();
});
sliderHeight.addChangeListener(e -> {
textHeight.setText("" + sliderHeight.getValue());
mainProgram.action();
});
panel.add(panelInput);
}
public JPanel getPanel() {
return panel;
}
public String getStringWeight() {
return textWeight.getText();
}
public String getStringHeight() {
return textHeight.getText();
}
}
А второй используется для отображения результатов:
import javax.swing.*;
import javax.swing.border.EmptyBorder;
import java.awt.*;
public class ResultPanel {
private final JPanel panel;
private final JLabel BMI;
private final JLabel information;
ResultPanel() {
panel = new JPanel();
panel.setLayout(new GridLayout(0, 1, 10, 10));
panel.setBorder(new EmptyBorder(10, 10, 10, 10));
BMI = new JLabel("Enter your weight and height in centimeters.");
BMI.setHorizontalAlignment(SwingConstants.CENTER);
information = new JLabel("");
information.setHorizontalAlignment(SwingConstants.CENTER);
panel.add(BMI);
panel.add(information);
}
public JPanel getPanel() {
return panel;
}
public void setResults(String IMC, String information) {
this.BMI.setText(IMC);
this.BMI.setForeground(Color.BLACK);
this.information.setText(information);
}
public void error(String s) {
this.BMI.setText(s);
this.BMI.setForeground(Color.RED);
this.information.setText("");
}
}
Никаких модульных тестов, потому что мы еще не продвинулись в моем классе.
Мы будем очень благодарны за любые советы или комментарии.
1 ответ
Соглашения об именах Java указать, что переменные и методы (включая поля / члены) должны иметь значение lowerCamelCase. Если они содержат аббревиатуру или что-то подобное, например HTTP, в них должна быть только начальная буква в верхнем регистре, если не в начале имени, например «httpClient» или «performHttpAction».
Соглашения об именах — важный инструмент, позволяющий быстро понять, на что они смотрят. 99,9999% Java-программистов сразу же предполагают, что имя «BMI» принадлежит какой-то константе.
И последнее, но не менее важное: вы используете «BMI» во всем приложении. По возможности следует избегать сокращений, относящихся к домену, поскольку они могут быть не совсем понятными, когда кто-то входит в домен.
private final double weight; //Kg.
private final int height; //cm.
Javadoc. Или, что еще лучше, сделайте его частью имени переменной weightInKg
. Или, что еще лучше, создайте тип для домена.
private static final NavigableMap<Double, String> grades = new TreeMap<>();
В качестве примечания, хотя это объявлено final
, содержимое все еще можно изменить.
Кроме того, возможно, это должно происходить из-за конфигурации в конце.
grades.put(0.0, "Insuficient Weight");
grades.put(18.5, "Normal Wight");
Опечатка.
public double getBMI() {
double heightMeters = height / 100.0;
return weight / (double) (heightMeters * heightMeters);
}
Кешируйте ИМТ (рассчитайте его один раз в конструкторе), он не изменится.
public String getGrade() {
Map.Entry<Double, String> gradeEntry = grades.floorEntry(getBMI());
if (gradeEntry == null) {
return "Incorrect Data";
}
return gradeEntry.getValue();
}
Тоже самое.
Коротко о дизайне API, ваш класс сейчас выглядит так:
public class PersonHealthData {
private final double weight; //Kg.
private final int height; //cm.
private static final NavigableMap<Double, String> grades = new TreeMap<>();
public double getWeight();
public int getHeight();
public double getBMI();
public String getGrade();
}
Итак, предположим, я хочу расширить этот класс. Пространства для маневра очень мало, если значения private
и final
. Однако, getWeight()
и getHeight()
не окончательные. Так что если я хочу вернуть другой weight
, например, поскольку я хочу, чтобы класс был изменяемым, я бы реализовал сеттер, имел бы другой private
член держит мой новый вес и отменяет getWeight()
. Теперь происходит то, что getBMI
и getGrade
вернет неверные значения, если я не реализую их повторно, однако их логика может быть мне неизвестна. Кроме того, я могу не сразу заметить эту проблему.
Возможные решения:
- Сделайте класс
final
. - Сделать
getWeight
иgetHeight
final
. Это эффективно исключает любую возможность расширения класса. - Сделать
weight
иheight
protected
и не окончательный. Это позволило бы эффективно расширить класс. - Сделать
getBMI
использоватьgetWeight
иgetHeight
вместо.
Итак, какое решение вы выберете, решать вам … время рассказа! Чаще всего я сталкивался с очень «свободным» использованием final
ключевое слово и столкнулся с большим количеством проблем, чем решил. Мне довольно часто бывает, что мне нужно расширить класс, чтобы либо задействовать, либо ограничить функциональность, потому что мой вариант использования не полностью соответствует реализации. А final
class исключает любую возможность сделать это, а это означает, что для этого необходимо найти другие способы.
Также возникает вопрос, чего вы хотите достичь. Лично мне сказали, что с помощью final
Буквально упростит чтение и сопровождение кода, с этим мнением я не могу согласиться. Потому что если методы и члены случайным образом protected
, private
и private final
и его необходимо время от времени менять, когда логика меняется, это больше похоже на решение, подобное культу карго, чем активное дизайнерское решение.
Хватит разглагольствовать, я могу часами говорить об этом, и это немного выходит за рамки этого обзора.
import javax.swing.*;
import java.awt.*;
В идеале следует избегать импорта подстановочных знаков, чтобы было легче увидеть, какие классы используются (возможны коллизии). Хорошая IDE также автоматически управляет импортом за вас.
MainProgram() {
Почему конструктор package-private
но класс public
?
window = new JFrame("BMI CAlculator");
Опечатка.
public static void main(String[] args) {
MainProgram mainProgram;
mainProgram = new MainProgram();
mainProgram.show();
}
Лично мне нравится сохранять main
в классе под названием Main
который больше ничего не содержит. Это позволяет очень легко найти точку входа в приложение.
results.error("Weight input is not a number");
Включите сообщение об исключении и значение, которое не удалось проанализировать в сообщении об ошибке, чтобы сократить время отладки.
this.BMI.setText(IMC);
this.BMI.setForeground(Color.BLACK);
this.information.setText(information);
Иногда вы используете «это», иногда нет, будьте последовательны.
Здесь вы слегка смешиваете обязанности. PersonHealthData
это модель, сервис и частичный вид провайдера в одном. Он содержит данные, вычисляет ИМТ и предоставляет строку, обращенную к пользователю.
Если все, что вы хотите сделать в своем приложении, это рассчитать ИМТ, PersonHealthData
совершенно не нужно, просто сделайте расчет в EntryPanel
и покончить с этим. В классе вообще нет необходимости.
Однако вы можете переместить расчет в BodyMassIndexCalculator
который имеет метод calculate
:
public class BodyMassIndexCalculator {
public double calculate(double heightInCentimeter, double weightInKilogram);
}
Теперь мы действительно хотели бы иметь класс этого значения, это означало бы, что нам нужна дополнительная информация. Создадим enum
который содержит классы.
public enum BodyMassIndexClass {
VERY_SEVERELY_UNDERWEIGHT,
SEVERLY_UNDERWEIGHT,
UNDERWEIGHT,
...;
}
Это дает нам хорошее начало. Предполагая, что эти классы ИМТ никогда не изменятся, мы напрямую свяжем их с соответствующими значениями:
public enum BodyMassIndexClass {
VERY_SEVERELY_UNDERWEIGHT(0, 15),
SEVERLY_UNDERWEIGHT(15, 16),
UNDERWEIGHT(16, 18.5),
...;
private final double from;
private final double to;
private BodyMassIndexClass(double from, double to) {
this.from = from;
this.to = to;
}
// TODO Getters for from and to.
public BodyMassIndexClass forIndex(double bodyMassIndexValue) {
for (BodyMassIndexClass bodyMassIndexClass : values()) {
if (bodyMassIndexClass.from <= bodyMassIndexValue && bodyMassIndexValue < bodyMassIndexClass.from) {
return bodyMassIndexClass;
}
}
return null;
}
}
Мы могли бы соединить это с красивым маленьким держателем:
public final class BodyMassIndex {
public final BodyMassIndexClass getBodyMassIndexClass();
public final double getValue();
}
Обратите внимание, что это final
поскольку он полностью лишен логики, он просто объединяет два значения.
Теперь это можно вернуть из нашего класса калькулятора.
В графическом интерфейсе пользователя теперь можно использовать переключатель для отображения соответствующего текста.