so here is my first question

C

chriske911

it's rather a lengthy question cause off the code I wrote:

package automaat;

/***************************************************************************
**/
import java.awt.*;
import java.awt.event.*;
import java.applet.*;
/***************************************************************************
**/
public class Automaten extends Applet implements ActionListener{
boolean RF=true;
int i,xPos=20;
Broodje SmosKaas=new Broodje();
Broodje SmosHesp=new Broodje();
Broodje Martino=new Broodje();
Broodje Pitta=new Broodje();
Button[] knopje;
Button knopje2;
String[] Opschrift = {"SmosKaas","SmosHesp","Martino","Pitta"};
TextField Input;
TextField Fouten;
/***************************************************************************
**/
public void init(){
setLayout(null);
SmosKaas.prijs = 190;
SmosHesp.prijs = 190;
Martino.prijs = 190;
Pitta.prijs = 220;
knopje2 = new Button("Bijvullen");
knopje2.setBounds(300,20,60,30);
add (knopje2);
knopje2.addActionListener(this);
knopje = new Button[4];
for (i=0;i<knopje.length;i++){
knopje = new Button(Opschrift);
knopje.setBounds((xPos+(85*i)),230,80,30);
add (knopje);
knopje.addActionListener(this);
}
Input = new TextField("");
Input.setBounds(160,30,80,40);
Input.setFont(new Font("Serif",Font.BOLD,24));
add (Input);
Fouten = new TextField ("Hier zie je boodschappen van mij aan u ");
Fouten.setBounds(5,270,390,20);
add (Fouten);
}
/***************************************************************************
**/
public void paint (Graphics g){
SmosKaas.DrawStock(g,20,100,60,Color.yellow);
SmosHesp.DrawStock(g,110,100,60,Color.gray );
Martino.DrawStock(g,200,100,60,Color.red );
Pitta.DrawStock(g,290,100,60,Color.black);
}
/***************************************************************************
**/
public class Broodje{
int prijs = 0, stock = 10;
String bericht;

public boolean Bestellen() {
if (stock > 0) {
stock--;
this.bericht = "";
return true;
}
else {
this.bericht = "Deze schuif is leeg";
return false;
}
}

public void BijVullen(int Aantal) {
if (this.stock >= 10) {
this.bericht = "deze schuif zit nog vol met broodjes";
}
else {
if ( (this.stock + Aantal) > 10) {
this.bericht = "Er zijn " + ( (this.stock + Aantal) - 10) +
" broodjes op overschot ";
this.stock = 10;
}
}
}

public void Bijvullen() {
if (this.stock >= 10) {
this.bericht = "deze schuif zit vol ";
}
else {
this.stock++;
this.bericht = "1 broodje erbij";
}
}

public void DrawStock(Graphics g, int X, int Y, int Breedte, Color
Kleur) {
g.setColor(Kleur);
g.fillRect(X, Y, Breedte, (this.stock * 10));
}

public String GetMessage() {
return this.bericht;
}

public void Display(TextField AnyTextField) throws Exception {
try {
AnyTextField.setText(this.bericht);
}
catch (Exception e) {
AnyTextField.setText("");
System.out.println("de volgende fout is opgetreden: " +
e.getMessage());
}
if (AnyTextField.getWidth() < this.bericht.length()) {
throw new Exception("tekstvak is vol te klein ");
}
}
}
/***************************************************************************
**/
public void actionPerformed(ActionEvent ae){
if (ae.getSource() == knopje2) {
if (RF == true) {
RF = false;
knopje2.setLabel("Bestellen ");
Fouten.setText(" U bent in de BroodjesBijVulModus ");
}
else {
RF = true;
knopje2.setLabel("Bijvullen ");
Fouten.setText("U kunt broodjes bestellen");
}
}
if (ae.getSource()==knopje[0]){
Afhandeling(SmosKaas,RF);
}
if (ae.getSource()==knopje[1]){
Afhandeling (SmosHesp,RF);
}
if (ae.getSource()==knopje[2]){
Afhandeling(Martino,RF);
}
if (ae.getSource()==knopje[3]){
Afhandeling(Pitta,RF);
}
repaint();
}
/***************************************************************************
**/
void Afhandeling( Broodje DCN,boolean YN){
int bedrag;
boolean RF = YN;
Broodje broodje = DCN;

try {
bedrag = Integer.parseInt(Input.getText());
}
catch (NumberFormatException nfe) {
System.out.println("Oeps");
bedrag = 0;
}

if (RF==true){
if (bedrag == 0) {
broodje.bericht = "er zit geen geld in de machine ";
Input.setText("" + bedrag);
}
else {
if (bedrag < broodje.prijs) {
broodje.bericht = "u komt " + (broodje.prijs - bedrag) +
" eurocent tekort voor deze aankoop ";
}
else {
if ((broodje.Bestellen())==true){
Input.setText("" + (bedrag - broodje.prijs));
broodje.bericht = "Smakelijk met uw broodje ";
}
}
}
}
else {
broodje.Bijvullen();
}
Fouten.setText(broodje.GetMessage());
//broodje.Display(Fouten);
}
}
/***************************************************************************
**/

this is the second try when the exam was over, the first was a real disaster

I was told to have a strange way of handling things so my question is:
what's wrong with this code or the way I use it ?

p.s.: I've left out most comment to shorten this message

thnx
 
R

Roedy Green

knopje2.setBounds(300,20,60,30);

normally you would use a layout. When you use the null layout, you
must remember to setBounds on every Component and Container, not just
teh location.
 
C

chriske911

that was my intention too, to clarify if a variabele should be true or
false,
I'm not looking for convention rules, I know them already but coming from VB
I mix conventions from time to time and it's not a one way mistake either
I was looking for pointers on how to build events and algoritmes and
structures (if, elseif, for, do ...)
this applet should compile without any problems but I was told I had a
strange way of handling events and to take strange turns with certain
methods

what do you think about the structure of this applet ?

thnx

Andy Flowers said:
or you can leave it with the form

if( variable == true )

to clarify the intent.

In my career I have come across too many times where people have written

if( variable )

and then discovered they 'forgot' to put the == false
 
M

Matt Humphrey

You have asked why your event handling seems unusual. In my opinion your
problem is that your event handler manually steps through each button to
determine which one was pressed. This means that although you keep the
buttons in an array, your event handling must explicitly index each one,
thereby defeating the purpose of using an array. If each button performs a
completely different operation I would declare them separately and attach a
specific handler to each. Alternatively, if you can unify the function
codes into a single list, I would send all the buttons to the same
dispatcher that would lookup the code in a hashtable or array and invoke
that. A key question for me is what would happen to your program if you
went from 4 buttons to 5? It seems to me that the array is inappropriate.

Consider the following changes:


chriske911 said:
it's rather a lengthy question cause off the code I wrote:

package automaat;

/***************************************************************************
**/
import java.awt.*;
import java.awt.event.*;
import java.applet.*;
/***************************************************************************
**/
public class Automaten extends Applet implements ActionListener{
boolean RF=true;
int i,xPos=20;
Broodje SmosKaas=new Broodje();
Broodje SmosHesp=new Broodje();
Broodje Martino=new Broodje();
Broodje Pitta=new Broodje();

There all your Broodje are handled the same way, you should derive them all
from a single main source. In fact, all of your rendering and UI can be
keyed to the Opscrift main array so that as you change just that single
array, your entire UI will change appropriately--recall the key question
about the array size above.

Map opscriftToBroodje;
Button[] knopje;
Button knopje2;
String[] Opschrift = {"SmosKaas","SmosHesp","Martino","Pitta"};
TextField Input;
TextField Fouten;
/***************************************************************************
**/
public void init(){
setLayout(null);
SmosKaas.prijs = 190;
SmosHesp.prijs = 190;
Martino.prijs = 190;
Pitta.prijs = 220;
knopje2 = new Button("Bijvullen");
knopje2.setBounds(300,20,60,30);
add (knopje2);
knopje2.addActionListener(this);

// Delete these lines
knopje = new Button[4];
for (i=0;i<knopje.length;i++){
knopje = new Button(Opschrift);
knopje.setBounds((xPos+(85*i)),230,80,30);
add (knopje);
knopje.addActionListener(this);


// Replace with
opscriftToBroodje = new HashMap ();
ActionListener opscriftListener = new ActionListener () {
public void actionPerformed (ActionEvent e) {
Broodje broodje = (Broodje)opscriftToBroodje.get (e.getSource ());
Afhandeling(broodje,RF);
}
};
for (int i = 0; i < Opscrift.length; ++i) {
Button b = new Button (Opschrift );
b.setBounds((xPos+(85*i)),230,80,30);
add (b);
b.addActionListener (opscriftListener);
}
}
Input = new TextField("");
Input.setBounds(160,30,80,40);
Input.setFont(new Font("Serif",Font.BOLD,24));
add (Input);
Fouten = new TextField ("Hier zie je boodschappen van mij aan u ");
Fouten.setBounds(5,270,390,20);
add (Fouten);
}
/***************************************************************************
**/
public void paint (Graphics g){
SmosKaas.DrawStock(g,20,100,60,Color.yellow);
SmosHesp.DrawStock(g,110,100,60,Color.gray );
Martino.DrawStock(g,200,100,60,Color.red );
Pitta.DrawStock(g,290,100,60,Color.black);
}
/***************************************************************************
**/
public class Broodje{
int prijs = 0, stock = 10;
String bericht;

public boolean Bestellen() {
if (stock > 0) {
stock--;
this.bericht = "";
return true;
}
else {
this.bericht = "Deze schuif is leeg";
return false;
}
}

public void BijVullen(int Aantal) {
if (this.stock >= 10) {
this.bericht = "deze schuif zit nog vol met broodjes";
}
else {
if ( (this.stock + Aantal) > 10) {
this.bericht = "Er zijn " + ( (this.stock + Aantal) - 10) +
" broodjes op overschot ";
this.stock = 10;
}
}
}

public void Bijvullen() {
if (this.stock >= 10) {
this.bericht = "deze schuif zit vol ";
}
else {
this.stock++;
this.bericht = "1 broodje erbij";
}
}

public void DrawStock(Graphics g, int X, int Y, int Breedte, Color
Kleur) {
g.setColor(Kleur);
g.fillRect(X, Y, Breedte, (this.stock * 10));
}

public String GetMessage() {
return this.bericht;
}

public void Display(TextField AnyTextField) throws Exception {
try {
AnyTextField.setText(this.bericht);
}
catch (Exception e) {
AnyTextField.setText("");
System.out.println("de volgende fout is opgetreden: " +
e.getMessage());
}
if (AnyTextField.getWidth() < this.bericht.length()) {
throw new Exception("tekstvak is vol te klein ");
}
}
}
/***************************************************************************
**/
public void actionPerformed(ActionEvent ae){
if (ae.getSource() == knopje2) {
if (RF == true) {
RF = false;
knopje2.setLabel("Bestellen ");
Fouten.setText(" U bent in de BroodjesBijVulModus ");
}
else {
RF = true;
knopje2.setLabel("Bijvullen ");
Fouten.setText("U kunt broodjes bestellen");
}
}

// Delete all of these
if (ae.getSource()==knopje[0]){
Afhandeling(SmosKaas,RF);
}
if (ae.getSource()==knopje[1]){
Afhandeling (SmosHesp,RF);
}
if (ae.getSource()==knopje[2]){
Afhandeling(Martino,RF);
}
if (ae.getSource()==knopje[3]){
Afhandeling(Pitta,RF);
} //
repaint();
}
/***************************************************************************
**/
void Afhandeling( Broodje DCN,boolean YN){
int bedrag;
boolean RF = YN;
Broodje broodje = DCN;

try {
bedrag = Integer.parseInt(Input.getText());
}
catch (NumberFormatException nfe) {
System.out.println("Oeps");
bedrag = 0;
}

if (RF==true){
if (bedrag == 0) {
broodje.bericht = "er zit geen geld in de machine ";
Input.setText("" + bedrag);
}
else {
if (bedrag < broodje.prijs) {
broodje.bericht = "u komt " + (broodje.prijs - bedrag) +
" eurocent tekort voor deze aankoop ";
}
else {
if ((broodje.Bestellen())==true){
Input.setText("" + (bedrag - broodje.prijs));
broodje.bericht = "Smakelijk met uw broodje ";
}
}
}
}
else {
broodje.Bijvullen();
}
Fouten.setText(broodje.GetMessage());
//broodje.Display(Fouten);
}
}
/***************************************************************************
**/

this is the second try when the exam was over, the first was a real disaster

I was told to have a strange way of handling things so my question is:
what's wrong with this code or the way I use it ?

p.s.: I've left out most comment to shorten this message

thnx

I have made these comments without understanding what Broodje, etc refer to.
If they have some kind of special meaning, you may need to consider that.
However, to the extent that they can be handled as named functions, you
should try to treat them all in the same manner.

Good luck,
Matt Humphrey (e-mail address removed) http://www.iviz.com/
 
S

Steve Horsley

it's rather a lengthy question cause off the code I wrote:

package automaat;
**/

this is the second try when the exam was over, the first was a real disaster

I was told to have a strange way of handling things so my question is:
what's wrong with this code or the way I use it ?

p.s.: I've left out most comment to shorten this message

thnx

Doesn't look too bad to me. Here are a few comments though:

Naming - please use a leading capital for class names and no leading
capital for method and variable names. This is a convention only, but it
is well followed and code that doesn't is much harder to read (by
programmers who DO follow the convention).

In a few places, I see things like
this.bericht = "";
where the use of "this" is not required.

I notice that in one place, you default bedrag to 0 if you cannot parse
the intended value properly. This may not be an appropriate thing to do -
an error message and do nothing may be safer, depending on the
application.

It is not a good idea to use a null layout - it makes the program less
platform independent - that's why layout managers exist.


People may have been commenting more on your program structure and logic
though. I didn't really understand what your program was about, so somone
who speaks whatever language the variable and method names are in might be
in a better position to comment there. I hope this helps a bit though.

Steve
 
C

Chris Smith

chriske911 said:
it's better to use "this.property" always so you'd never forget it in any
class, it doesn't hurt the progam and it prevents confusion between class
and global variables who have the same name or so I've read in a book or
guide somewhere

I'd question the book or guide. First of all, there's no such thing as
global variables in Java, so the confusion you're trying to eliminate
doesn't really exist in the first place. The only time that "this." is
required is when an instance variable conflicts in name with a local
variable... something that should probably be avoided anyway (though I
must admit I do it for mutators when the the name is so short it can't
be simplified for the param name).

Having "this." all over your code is, IMHO, sure to make your code more
cumbersome to read. It's likely to mislead those reading the code,
causing them to look for a justification for the unnecessary language.
I don't see a lot of benefit. So, I wouldn't do it.
I do try to follow convention but having coded in VB and C++ and now Java
makes me switch conventions from time to time,

it's not one way, I make the same mistake by using Java convention or code
in VB or C++

Well, Java is a little bit unique among the languages you work in, in
that naming conventions are practically universal. While there are
several conventions widely in use among C++ and VB shops, Java has one
naming convention. Not using that one convention is quite frowned upon.
There are advantages to that convention that only work if practically
everyone uses it.

--
www.designacourse.com
The Easiest Way to Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,764
Messages
2,569,567
Members
45,041
Latest member
RomeoFarnh

Latest Threads

Top