Win a copy of Kotlin in Action this week in the Kotlin forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic

How is this design?  RSS feed

 
Sean Magee
Ranch Hand
Posts: 69
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hey There Ranchers!

I know this may be a bit too much to ask,
but I would like for someone to check my program
if they could,
and to tell me a few things i could improve.
This program combines my knowledge of Java
with OOP concepts at the moment(excluding some stuff liek inheritance),
and before I take the next step, I would like to see
if there was anything i could have done better
(which i know there is).
I don't want to code incoherently (spaghetti code),
and I want to be effecient and simple as possible.
Its sort of a booking program.
oh yeah, dont be nice


So please, any suggestions on my program please let me know.
Everything does compile and runs fine.





Thanks in Advance!!



[ March 09, 2005: Message edited by: Sean Magee ]
 
Mark Spritzler
ranger
Sheriff
Posts: 17309
11
IntelliJ IDE Mac Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I'll try to help a little bit.

1). Read Sun'sJava Coding Conventions. Your indentation in the post is off, follow naming standards.

2).


Your code here will only check the first seat. So if three people are booked, there is still a seat left, but this method will return false. Have your learned Collection classes here. You could use and ArrayList, and just check the length, if it is 4 then it is full.

3). I'd make all your instance variables private to ensure proper encapsulation.

4). In Bus.addCust, you are always resetting your seatcounter to 1 each time, What is your intention with the seatCounter (proper naming standard here)? It is only used in this method, so I would declare the variable inside the method instead of making it a static member of the Class. Probably the same with shuttleCounter.

That's just a few to get your started.

Have fun.

Mark
 
Jeff Langr
author
Ranch Hand
Posts: 799
5
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
a few off the top of my head:

- in printOrder, use a StringBuffer or StringBuilder (5.0) to aggregate the fullorder text.
- isolate user interface from your model classes. For example, Bus shouldn't use JOptionPanes. Push as much "business" logic into Bus and other model classes as possible.
- don't use hardcoded literals ("magic numbers"), such as 4.

-j-
 
Ernest Friedman-Hill
author and iconoclast
Sheriff
Posts: 24217
38
Chrome Eclipse IDE Mac OS X
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Don't use "static" except for its intended purpose of sharing something among all instances of a class. You've used "static" here in a classic "newbie" way, basically making everything in the Talk class static so they're easy to get to from "main()". Instead of doing this, make everything non-static, and then replace main's body with

new Talk().getWelcome();

i.e., create a Talk object to work with.
 
Ilja Preuss
author
Sheriff
Posts: 14112
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I'd like to comment, but your code is nearly unreadable in my browser because of the heavy indentation. If you could fix that...

You can edit your post by using the "pen and paper" icon at its top.
 
Sean Magee
Ranch Hand
Posts: 69
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Would like to mention on how much i appreciate all of your comments.

They are extremely helpful. I changed the indentions, maybe that will help
[ March 09, 2005: Message edited by: Sean Magee ]
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!