Is my book bad or am I doing 'wrong'?

**Catgroove** · November 16th, 2010, 03:56 AM

I'm reading this swedish book about programming but I have this feeling that it teaches badly.

One example is this code. It's a program that asks how long your call lasted, then it asks if it was on daytime or nighttime, then it gives you the total cost for that call.

Here's how I wrote it. Instead of just multiplying the prices in the final message I assigned them to variables so you can change them later. I felt like this was a good approach because I feel it's always good to be able to change your numbers in one place if you've used them in several occasions.

import javax.swing.*;
 
public class Samtal {
 
	public static void main(String[] args) {
 
		double dayTimePrice = 5.50;
		double nightTimePrice= 0.55;
		double openPrice = 0.40;
		double totalPrice;
		int minutes = new Integer (JOptionPane.showInputDialog("How many minutes did the call last?"));
		int time = new Integer (JOptionPane.showConfirmDialog(null, "Did the call take place on daytime?"));
 
		if (time == 0) {
			totalPrice = (minutes * dayTimePrice) + openPrice;
			JOptionPane.showMessageDialog(null, "The total cost is " + totalPrice);
		}
		else if (time == 1) {
			totalPrice = (minutes * nightTimePrice) + openPrice;
			JOptionPane.showMessageDialog(null, "The total cost is" + totalPrice );
		}
		else {
			System.exit(0);
		}
 
		System.exit(0);
 
 
	}
 
}

Here's how the author wrote it:

import javax.swing.*;
 
public class Mobil3 {
  public static void main (String[] arg) {
    String s = JOptionPane.showInputDialog("Minutes of the call?");
    int min = Integer.parseInt(s);
    int answer = JOptionPane.showConfirmDialog(null, "Daytime?");
    if (answer == 0)
      JOptionPane.showMessageDialog(null, "Cost: " + (0.4+min*5.50));
    else if (answer == 1)
      JOptionPane.showMessageDialog(null, "Cost: " + (0.4+min*0.55));
    System.exit(0);
  }
}

Note: I had to translate the code for you to understand it so it may look a bit stupid.

I know his code was better in terms of how much we both wrote but I still felt like mine was the 'right' way to do it. The reason I'm having this thoughts is because I watched this Standford lecture on youtube about Java and he kept repeating that your code should be very easy to edit and divide it into small parts. That's what I tried to do.

**Darryl.Burke** · November 16th, 2010, 05:13 AM

Just how verbose you make your code is at least partly a matter of personal choice. Code that is either too verbose ro too terse can be hard to maintain. Generally speaking, experienced programmers would rather deal with code that is reasonably terse. Excess verbosity is only favored by those new to the field.

Assigning a int return value to a new Integer(...) is just wasteful, since the Integer is immediately assigned to a int variable through auto(un)boxing. Computing and assigning totalPrice in the context of if and else could only be meaningful if the value is used once, outside the if/else blocks.

        if (time == 0) {
            totalPrice = (minutes * dayTimePrice) + openPrice;
        }
        else if (time == 1) {
            totalPrice = (minutes * nightTimePrice) + openPrice;
        }
        JOptionPane.showMessageDialog(null, "The total cost is " + totalPrice);

I have a major issue with two lines of code seen in both your code and the author's.

if (answer == 0)
:
else if (answer =1)

Is that 'Yes' or 'No'? That JOptionPane.showConfirmDialog returns 0 when the 'Yes' option is selected is an implementation detail of JOptionPane, and any use of the numeric value outside JOptionPane.java is what is called using a 'magic number'. The correct way is to test against the class constant.

if (answer == JOptionPane.YES_OPTION)
:
else if (answer == JOptionPane.NO_OPTION)

db

edit The 'else' after if ... else if is redundant. If the showConfirmDialog returns CANCEL_OPTION, neither 'if' condition will be satisfied and control will fall through to the next statement after the if .. else if blocks, which is exit(0) -- the same as in the last else condition.

**Catgroove** · November 16th, 2010, 07:49 AM

Originally Posted by Darryl.Burke

Just how verbose you make your code is at least partly a matter of personal choice. Code that is either too verbose ro too terse can be hard to maintain. Generally speaking, experienced programmers would rather deal with code that is reasonably terse. Excess verbosity is only favored by those new to the field.

Assigning a int return value to a new Integer(...) is just wasteful, since the Integer is immediately assigned to a int variable through auto(un)boxing. Computing and assigning totalPrice in the context of if and else could only be meaningful if the value is used once, outside the if/else blocks.

        if (time == 0) {
            totalPrice = (minutes * dayTimePrice) + openPrice;
        }
        else if (time == 1) {
            totalPrice = (minutes * nightTimePrice) + openPrice;
        }
        JOptionPane.showMessageDialog(null, "The total cost is " + totalPrice);

I have a major issue with two lines of code seen in both your code and the author's.

if (answer == 0)
:
else if (answer =1)

Is that 'Yes' or 'No'? That JOptionPane.showConfirmDialog returns 0 when the 'Yes' option is selected is an implementation detail of JOptionPane, and any use of the numeric value outside JOptionPane.java is what is called using a 'magic number'. The correct way is to test against the class constant.

if (answer == JOptionPane.YES_OPTION)
:
else if (answer == JOptionPane.NO_OPTION)

db

edit The 'else' after if ... else if is redundant. If the showConfirmDialog returns CANCEL_OPTION, neither 'if' condition will be satisfied and control will fall through to the next statement after the if .. else if blocks, which is exit(0) -- the same as in the last else condition.

He never mentioned the YES_OPTION and NO_OPTION in the book, only 0, 1 and 2.

So what you're saying is his example was the "better" one for an experienced programmer and thats how you would approach it?

..and when you're talking about assigning a int to a new Integer do you mean this line:
int minutes = new Integer? Because that's converting it from a string or whatever so you don't have to assign a string first then parseInt.

**Darryl.Burke** · November 16th, 2010, 08:49 AM

Integer#parseInt(String) is the standard idiom for obtaining a int from a String.

Integer#valueOf(String) is the standard idiom for obtaining a Integer from a String, and is used under the covers by autoboxing.

new Integer(String) is rarely needed, and only when the nature of the algorithm demands that each Integer object so constructed will be a separate instance i.e. the built-in caching mechanism won't come into play.

You don't have to "assign a String" before parsing it any more or less than you don't "assign a String" before feeding it to the Integer constructor.

int minutes = Integer.parseInt(JOptionPane.showInputDialog("How many minutes did the call last?"));

db

**Catgroove** · November 16th, 2010, 09:13 AM

Originally Posted by Darryl.Burke

Integer#parseInt(String) is the standard idiom for obtaining a int from a String.

Integer#valueOf(String) is the standard idiom for obtaining a Integer from a String, and is used under the covers by autoboxing.

new Integer(String) is rarely needed, and only when the nature of the algorithm demands that each Integer object so constructed will be a separate instance i.e. the built-in caching mechanism won't come into play.

You don't have to "assign a String" before parsing it any more or less than you don't "assign a String" before feeding it to the Integer constructor.

int minutes = Integer.parseInt(JOptionPane.showInputDialog("How many minutes did the call last?"));

db

Thank you for clearing that up. Would you mind writing the program "your" way? Just curious how you would do it.

**Darryl.Burke** · November 16th, 2010, 12:20 PM

That won't help you

import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JCheckBox;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JSpinner;
import javax.swing.JTextField;
import javax.swing.SpinnerNumberModel;
import javax.swing.SwingUtilities;
import javax.swing.event.ChangeEvent;
import javax.swing.event.ChangeListener;
 
public class CallCost {
 
  final static double BASE_RATE = 0.4;
  static final double DAY_RATE = 5.5;
  static final double NIGHT_RATE = 0.55;
  //
  protected SpinnerNumberModel model = new SpinnerNumberModel(0, 0, 1000, 1);
  protected JSpinner durationSpinner = new JSpinner(model);
  protected JCheckBox dayCheckBox = new JCheckBox("Day");
  protected JTextField chargeTextField = new JTextField();
 
  public static void main(String[] args) {
    SwingUtilities.invokeLater(new Runnable() {
 
      @Override
      public void run() {
        new CallCost().makeUI();
      }
    });
  }
 
  public void makeUI() {
    durationSpinner.addChangeListener(new ChangeListener() {
 
      @Override
      public void stateChanged(ChangeEvent e) {
        computeCharge();
      }
    });
    dayCheckBox.addActionListener(new ActionListener() {
 
      @Override
      public void actionPerformed(ActionEvent e) {
        computeCharge();
      }
    });
    chargeTextField.setEditable(false);
    chargeTextField.setHorizontalAlignment(JTextField.RIGHT);
 
    JPanel panel = new JPanel(new GridBagLayout());
    GridBagConstraints gbc = new GridBagConstraints();
    gbc.fill = GridBagConstraints.BOTH;
    gbc.gridx = 0;
    gbc.gridy = 0;
    panel.add(new JLabel("Duration: "), gbc);
 
    gbc.gridx = 1;
    panel.add(durationSpinner, gbc);
 
    gbc.gridx = 2;
    panel.add(dayCheckBox, gbc);
 
    gbc.gridx = 0;
    gbc.gridy = 1;
    panel.add(new JLabel("Charge: "), gbc);
 
    gbc.gridx = 1;
    gbc.gridwidth = 2;
    panel.add(chargeTextField, gbc);
 
    computeCharge();
 
    JFrame frame = new JFrame();
    frame.add(panel);
    frame.pack();
 
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    frame.setLocationRelativeTo(null);
    frame.setVisible(true);
  }
 
  protected void computeCharge() {
    double charge = BASE_RATE
            + ((Integer) durationSpinner.getValue()).intValue()
            * (dayCheckBox.isSelected() ? DAY_RATE : NIGHT_RATE);
    chargeTextField.setText(String.format("%10.2f", charge));
  }
}

Note: in real world applications, floating point primitives (float / double) are never to be used for currency calculations. Either use 100 X the value and an integral type (byte / char / int / long) and format the output to insert a decimal point before the last two characters, or use BigDecimal.

db

**Catgroove** · November 16th, 2010, 02:51 PM

Originally Posted by Darryl.Burke

That won't help you

import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JCheckBox;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JSpinner;
import javax.swing.JTextField;
import javax.swing.SpinnerNumberModel;
import javax.swing.SwingUtilities;
import javax.swing.event.ChangeEvent;
import javax.swing.event.ChangeListener;
 
public class CallCost {
 
  final static double BASE_RATE = 0.4;
  static final double DAY_RATE = 5.5;
  static final double NIGHT_RATE = 0.55;
  //
  protected SpinnerNumberModel model = new SpinnerNumberModel(0, 0, 1000, 1);
  protected JSpinner durationSpinner = new JSpinner(model);
  protected JCheckBox dayCheckBox = new JCheckBox("Day");
  protected JTextField chargeTextField = new JTextField();
 
  public static void main(String[] args) {
    SwingUtilities.invokeLater(new Runnable() {
 
      @Override
      public void run() {
        new CallCost().makeUI();
      }
    });
  }
 
  public void makeUI() {
    durationSpinner.addChangeListener(new ChangeListener() {
 
      @Override
      public void stateChanged(ChangeEvent e) {
        computeCharge();
      }
    });
    dayCheckBox.addActionListener(new ActionListener() {
 
      @Override
      public void actionPerformed(ActionEvent e) {
        computeCharge();
      }
    });
    chargeTextField.setEditable(false);
    chargeTextField.setHorizontalAlignment(JTextField.RIGHT);
 
    JPanel panel = new JPanel(new GridBagLayout());
    GridBagConstraints gbc = new GridBagConstraints();
    gbc.fill = GridBagConstraints.BOTH;
    gbc.gridx = 0;
    gbc.gridy = 0;
    panel.add(new JLabel("Duration: "), gbc);
 
    gbc.gridx = 1;
    panel.add(durationSpinner, gbc);
 
    gbc.gridx = 2;
    panel.add(dayCheckBox, gbc);
 
    gbc.gridx = 0;
    gbc.gridy = 1;
    panel.add(new JLabel("Charge: "), gbc);
 
    gbc.gridx = 1;
    gbc.gridwidth = 2;
    panel.add(chargeTextField, gbc);
 
    computeCharge();
 
    JFrame frame = new JFrame();
    frame.add(panel);
    frame.pack();
 
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    frame.setLocationRelativeTo(null);
    frame.setVisible(true);
  }
 
  protected void computeCharge() {
    double charge = BASE_RATE
            + ((Integer) durationSpinner.getValue()).intValue()
            * (dayCheckBox.isSelected() ? DAY_RATE : NIGHT_RATE);
    chargeTextField.setText(String.format("%10.2f", charge));
  }
}

Note: in real world applications, floating point primitives (float / double) are never to be used for currency calculations. Either use 100 X the value and an integral type (byte / char / int / long) and format the output to insert a decimal point before the last two characters, or use BigDecimal.

db

Thanks, but I meant with the knowledge I've got.

Thread: Is my book bad or am I doing 'wrong'?

LinkBack

Thread Tools

Display

Is my book bad or am I doing 'wrong'?

Related threads:

Re: Is my book bad or am I doing 'wrong'?

Re: Is my book bad or am I doing 'wrong'?

Re: Is my book bad or am I doing 'wrong'?

Re: Is my book bad or am I doing 'wrong'?

Re: Is my book bad or am I doing 'wrong'?

Re: Is my book bad or am I doing 'wrong'?

Similar Threads

Address Book Help

Java Book

Array of contacts (address book)

JSP E-BOOK URGENT PLEASE

Help with JAVA (Grade Book)