current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 6 down vote favorite
1

I was told to create a calculator design on AWT, which is also my first project on AWT. I wrote the program and submitted it. I was then told that my program contains around 70 lines of code, and I should try to write less code. How can I shorten this code?

import java.awt.*;
class Demo{
    Frame f;
    TextField tf;
    Button add,sub,mul,div,find,zero,num1,num2,num3,num4,num5,num6,num7,num8,num9;
    Button num0,decimal,per;
    Demo(String s)
    {
        f=new Frame(s);
        tf=new TextField("0");
        tf.setBounds(20, 40, 205, 40);
        f.add(tf);
        add=new Button("+");
        sub=new Button("-");
        div=new Button("/");
        mul=new Button("x");
        find=new Button("=");
        zero=new Button("C");
        num1=new Button("1");
        num2=new Button("2");
        num3=new Button("3");
        num4=new Button("4");
        num5=new Button("5");
        num6=new Button("6");
        num7=new Button("7");
        num8=new Button("8");
        num9=new Button("9");
        num0=new Button("0");
        per=new Button("%");
        decimal=new Button(".");
        mul.setBounds(125,100,50,40);
        add.setBounds(70,100,50,40);
        sub.setBounds(15, 100, 50, 40);
        div.setBounds(180, 100, 50, 40);
        find.setBounds(180,250,50,88);
        zero.setBounds(180, 150, 50, 40);
        num9.setBounds(15,150,50,40);
        num8.setBounds(70,150,50,40);
        num7.setBounds(125,150,50,40);
        num6.setBounds(15,200,50,40);
        num5.setBounds(70,200,50,40);
        num4.setBounds(125,200,50,40);
        num3.setBounds(15,250,50,40);
        num2.setBounds(70,250,50,40);
        num1.setBounds(125,250,50,40);
        num0.setBounds(15,300,105,40);
        decimal.setBounds(125,300,50,40);
        per.setBounds(180,200,50,40);
        f.add(per);
        f.add(add);
        f.add(mul);
        f.add(div);
        f.add(sub);
        f.add(find);
        f.add(zero);
        f.add(num1);
        f.add(num2);
        f.add(num3);
        f.add(num4);
        f.add(num5);
        f.add(num6);
        f.add(num7);
        f.add(num8);
        f.add(num9);
        f.add(num0);
        f.add(decimal);
        f.setLayout(null);
        f.setSize(250,350);
        f.setVisible(true);
    }
    public static void main(String[] args)
    {
        new Demo("Calculator");
    }

}
share|improve this question
    
Even though the program seems far from complete, I think there are some things that we can review here. Thanks for coming here, you will most likely get an answer soon! –  Simon Forsberg♦ Jan 31 '14 at 10:24
    
Thank you Simon –  Sajal Arora Jan 31 '14 at 10:35
    
Whenever you start seeing yourself repeating code, you should think of ways to abstract that portion. @amon made a great suggestion. Single Responsibility and DRY (Don't repeat yourself) are really great concepts to look into. –  harsimranb Jan 31 '14 at 21:44

3 Answers 3

up vote 14 down vote accepted

Write a helper method, e.g.

static Button createButton(Frame f, String label, int x0, int y0, int width, int height) {
    Button b = new Button(label);
    b.setBounds(x0, y0, width, height);
    f.add(b);
    return b;
}

Then your code would simplify to lines like:

createButton(f, "+", 70, 100, 50, 40);

which is a reduction of roughly 2/3.

Of course this does not address the problem that you are trying to do pixel-exact layouts. You should rather be dropping elements on a grid which can be resized freely. (However, it has been a long time since I've used AWT so I can't give an example).

share|improve this answer
    
Thanks for suggestion @amon –  Sajal Arora Jan 31 '14 at 10:28
1  
+1 this would be a very readable and maintainable solution. 1 line per button looks pretty elegant to me. I may have not passed in frame and instead just returned the button, then each line would read f.Add(SimpleButton("+", 70,100,50,40)) but either way looks pretty good. –  deepee1 Jan 31 '14 at 15:09
3  
Returning the button already added to the frame has an advantage: It allows to assign the button to a Button variable for later reference (adding action listeners etc.), without a second line for adding it to the frame. Like so: Button add = createButton(f, "+", 70, 100, 50, 40); –  Christian Semrau Jan 31 '14 at 16:44
    
+1 nice answer! –  Mat's Mug♦ Feb 2 '14 at 0:09
up vote 8 down vote

In addition to what @amon said, I suggest that you use an array for storing the numerical buttons

Button[] numbers = new Button[10];

numbers[9].setBounds(15, 150,50, 40);
numbers[8].setBounds(70, 150,50, 40);
numbers[7].setBounds(125,150,50, 40);
numbers[6].setBounds(15, 200,50, 40);
numbers[5].setBounds(70, 200,50, 40);
numbers[4].setBounds(125,200,50, 40);
numbers[3].setBounds(15, 250,50, 40);
numbers[2].setBounds(70, 250,50, 40);
numbers[1].setBounds(125,250,50, 40);
numbers[0].setBounds(15, 300,105,40);

The positions of the 1-9 numbers can be computed mathematically which makes it possible to create these buttons in a for-loop. I have not tested this calculation, but I believe it is correct:

numbers[0] = new Button("0");
numbers[0].setBounds(15, 300,105,40);
for (int i = 1; i <= 9; i++) {
    numbers[i] = new Button(Integer.toString(i));
    int x = (i - 1) % 3;
    int y = i / 3;
    numbers[i].setBounds(125 - 55 * x, 250 - 50 * y, 50, 40);
}

However, it is really a good idea to put these buttons into a Layout. Using exact pixel values is not recommended as it will not be scalable at all if you want to resize your form.

In addition to this, there is also some other things that I suggest you improve:

  • All the variables in your class can be private, and prefferably also final
  • Use spaces around the assignment operator, f = new Frame(s); looks better
  • Naming a TextField to tf provides not much information about the variable. What is it used for? input can be a better name.
  • Don't be afraid of using variable names longer than four characters, percent is better than per for example.
  • I recommend to also use spaces after a comma. Button add, subtract, multiply would be better (also improving the variable names)
share|improve this answer
    
= is not character, it's assignment operator. –  Anirban Nag 'tintinmj' Jan 31 '14 at 14:38
    
@tintinmj Technically, i'd say that it's a character also - at least in the computer world :) –  Simon Forsberg♦ Jan 31 '14 at 14:41
    
@SimonAndréForsberg I think the OP has the primitive char in mind. –  11684 Feb 1 '14 at 7:59
up vote 2 down vote

First thing what have written is nice try for the first time programer but you should use layout instead of using null out.

use 2 panel & add in to frame in border layout.(top & center)

in top add textField.

& 2nd panel use grid layout & simply add them. this might decrease your code.

share|improve this answer
    
Thanks for suggestion but my teacher yet not explain the concept of Layout so when I'll understand this layout concept. I'll Definitely use your suggestion. –  Sajal Arora Jan 31 '14 at 10:33
4  
@user1444692 although you haven't been taught about layouts yet, there's no harm in doing a bit of extra-curricular study and working them out. I'm sure there are plenty of AWT books out there to get you started ;) –  Joe Jan 31 '14 at 14:52

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.