Skip to main content
Code Review

Return to Answer

added 5 characters in body
Source Link
private JButton createButton(String text, ActionListener action){
 JButton button = new JButton(text);
 button.addActionListener(action);
 button.setFocusable(false);
 return button;
}
private JButton createMemoryEraseButton(){
 return createButton("ME",(e)->memoryStore.resetStoredValue());
}
private JButton createMemoryRecallButton(){
 return createButton("MR",(e)->{
 display = screen.getText();
 screen.setText(display + "" + memoryStore.getStoredValue());
 });
}
// ....
private voidJButton[] createOperationButtons() {
 operationButtons = new JButton[] {
 createMemoryEraseButton(),
 createMemoryRecallButton(),
 // ....
 };
 return operationButtons;
}
private JButton createButton(String text, ActionListener action){
 JButton button = new JButton(text);
 button.addActionListener(action);
 button.setFocusable(false);
 return button;
}
private JButton createMemoryEraseButton(){
 return createButton("ME",(e)->memoryStore.resetStoredValue());
}
private JButton createMemoryRecallButton(){
 return createButton("MR",(e)->{
 display = screen.getText();
 screen.setText(display + "" + memoryStore.getStoredValue());
 });
}
// ....
private void createOperationButtons() {
 operationButtons = new JButton[] {
 createMemoryEraseButton(),
 createMemoryRecallButton(),
 // ....
 };
}
private JButton createButton(String text, ActionListener action){
 JButton button = new JButton(text);
 button.addActionListener(action);
 button.setFocusable(false);
 return button;
}
private JButton createMemoryEraseButton(){
 return createButton("ME",(e)->memoryStore.resetStoredValue());
}
private JButton createMemoryRecallButton(){
 return createButton("MR",(e)->{
 display = screen.getText();
 screen.setText(display + "" + memoryStore.getStoredValue());
 });
}
// ....
private JButton[] createOperationButtons() {
 operationButtons = new JButton[] {
 createMemoryEraseButton(),
 createMemoryRecallButton(),
 // ....
 };
 return operationButtons;
}
added 1298 characters in body
Source Link

You could even go one step further and move the method createOperationButtons() and all the create??Button() methods in to a separate class.

Off cause, all the objects these methods work on you would have to pass in as prarameters either to the constructor of the new class or the createOperationButtons() method and subsequently to the create??Button() methods as needed.

You could even go one step further and move the method createOperationButtons() and all the create??Button() methods in to a separate class.

Off cause, all the objects these methods work on you would have to pass in as prarameters either to the constructor of the new class or the createOperationButtons() method and subsequently to the create??Button() methods as needed.

added 1298 characters in body
Source Link

I have to do it 18 times in one method it makes the method very big and ugly. There has to be a way of making it nice and clean. – Alex. M

You cannot avoid having to specify the different behavior for the operator buttons.

In your first solution the ugliness was the if/else cascade in your actionPerformed() method.

With my suggestion you could at least ease the readers pain by having an additional method for each operator button:

private JButton createButton(String text, ActionListener action){
 JButton button = new JButton(text);
 button.addActionListener(action);
 button.setFocusable(false);
 return button;
}
private JButton createMemoryEraseButton(){
 return createButton("ME",(e)->memoryStore.resetStoredValue());
}
private JButton createMemoryRecallButton(){
 return createButton("MR",(e)->{
 display = screen.getText();
 screen.setText(display + "" + memoryStore.getStoredValue());
 });
}
// ....
private void createOperationButtons() {
 operationButtons = new JButton[] {
 createMemoryEraseButton(),
 createMemoryRecallButton(),
 // ....
 };
}

I have to do it 18 times in one method it makes the method very big and ugly. There has to be a way of making it nice and clean. – Alex. M

You cannot avoid having to specify the different behavior for the operator buttons.

In your first solution the ugliness was the if/else cascade in your actionPerformed() method.

With my suggestion you could at least ease the readers pain by having an additional method for each operator button:

private JButton createButton(String text, ActionListener action){
 JButton button = new JButton(text);
 button.addActionListener(action);
 button.setFocusable(false);
 return button;
}
private JButton createMemoryEraseButton(){
 return createButton("ME",(e)->memoryStore.resetStoredValue());
}
private JButton createMemoryRecallButton(){
 return createButton("MR",(e)->{
 display = screen.getText();
 screen.setText(display + "" + memoryStore.getStoredValue());
 });
}
// ....
private void createOperationButtons() {
 operationButtons = new JButton[] {
 createMemoryEraseButton(),
 createMemoryRecallButton(),
 // ....
 };
}
Source Link
Loading
lang-java

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