IWETHEY v. 0.3.0 | TODO
1,095 registered users | 0 active users | 0 LpH | Statistics
Login | Create New User
IWETHEY Banner

Welcome to IWETHEY!

New Couple comments
Just from a conventional style thing ie what everybody does - var names and package names start with a lower case letter. Only class names are fully capitalized. I always start my member var names with an underscore so I don't have to keep going this.name = name; instead I say _name = name and I can tell at a glance whats an ivar and whats a param or temp.

That's niggly though.

From a design point of view let me ask a couple questions.

SalesOrder derives from CustomerPO. Is a CustomerPO ever of use by itself? If not, why isn't CustomerPO declared abstract? Ditto for Person and other superclasses. Also, why subclass at all? How many subtypes does CustomerPO have? Are the similarities important. Is it important to be able to deal with things at the CustomerPO level ignoring the true type of the object. If not, do away with the superclass. As a rule of thumb, BTW, I try like hell to avoid any inheritance at all in my business entity model because its a major PITA to map inheritance into a RDBMS and in general its more trouble than its worth. Inheritance in general is much overused.

New thing:
this ctor:
public Customer(String newFirstName,
String newLastName,
String newCompanyName,
String newAccountNumber,
Address newBillingAddress,
Address newShippingAddress,
String newPhoneNumber,
int newBuyerID,
double newDiscount,
boolean newTaxExempt,
String newTaxCertNumber,
String newInstructions,
String newNotes)

is an unholy nightmare and the odds of any application programmer correctly filling in a parameter list like that are roughly <number of params factorial> to 1. Better to let the developer construct the thing with a few key fields and let them set the other stuff via accessors. Its just easier to remember. If this is the only ctor, nobody is ever going to manage to create one of these things - its too hard.

You use eager initialization here:

public SalesOrder(CustomerPO newCustomerPO)
{
\tif (newCustomerPO == null)
\t{
\t setPO(new CustomerPO());
\t}
\telse
\t{
\t setPO(newCustomerPO);
\t}
}

which I assume means that you gotta have a CustomerPO. OTOH, its definitely possible to create one of these objects in some GUI and then never use it (because the user hits cancel or something), which is fine. So you throw away an object - but why throw away two? Use lazy initialization instead. Let the PO be null and create one when its asked for. ie

public CustomerPO getPO()
{
if(associatedPurchaseOrder == null) setPO(new CustomerPO());
\treturn associatedPurchaseOrder;
}

The efficiency freaks will holler "but you do that test for null over and over again! Yeah so? That test is super super cheap. In fact, I defy you to write two versions of the program and get that test's impact to show up in your performance profile.

Other question/comment. You have these CollectionOf.... things that I assume are an attempt to do typesafe collections. OK, sort of. Thing is - if you ever have to give one of these to some foreign API, your collection class is not going to be the right type. You'll have to convert. You may have to anyway but its worth a thought - what data type is going to be most convenient to pass around? A typesafe toArray might be helpful.

Other thing - you declare the implementing class to be a LinkedList. Why?

Problem #1 - the interface for the collection implementation class you've specified is too specific. Declare the ivar and all call interfaces to be simply List. Then you can tune and you only have to change the line that says new <concrete List class> to change the implementation. Never declare a variable of type concrete collection subclass - only of type abstract collection interface. Same for params.

Problem #2 - is it OK to have a Buyer appear twice in CollectionOfBuyer? Is the order of the items in the collection important? My guess is no, and no. So you really want a Set don't you. Declare the ivar of type Set and then construct some concrete Set class to initialize it. Now you get sanity checking for free against double adds.

These are just things I found in about 5 minutes - there may be other things but this is what jumps out at me.

New Thanks
Sales orders and Customer Purchase Orders are related - you can't initiate a sales order without a customer purchase order. Sales Orders must have a unique Sales Order ID, and Customer POs don't, since different customers may have ID collisions - Otherwise, they're identical.

I'll put in some other creators.

Collection issues are mainly due to poor book/unfamiliarity with language/lack of instructor assistance.

I did do some code updates last night, but forgot to replace the old version with the new - so I'll be doing that tonight, along with a (very primitive) GUI. Right now, the tendrils from the GUI are starting to reach out towards the tendrils of the data stores, but neither are seeing each other yet.
"He who fights with monsters might take care lest he thereby become a monster. And if you gaze for long into an abyss, the abyss gazes also into you." - Friedrich Nietzsche
New questions
>> Sales orders and Customer Purchase Orders are related - you can't initiate a sales order without a customer purchase order. Sales Orders must have a unique Sales Order ID, and Customer POs don't, since different customers may have ID collisions - Otherwise, they're identical. <<

A "sales order" is like a draft purchase order, no? I see no reason to use inheritance there. A purchase order is not "a kind of" sales order.

But, anyhow, why are you allowing "ID collisions"? Why have ID's if they collide? Am I missing something here? Are they internal sales orders, or external? Normally, outsiders don't see internal sales orders, except for stuff they order.

And, why have unique sales order ID's, but not unique purchase order ID's?


________________
oop.ismad.com
New Purchase Order ID is specified by the purchaser, i.e...
the customer. Customers use POs to track orders they make, the order approval and budgeting, order receiving, and eventually payment.

Customers do not coordinate their P. O. IDs among themselves to make your life simple.
Alex

Men never do evil so completely and cheerfully as when they do it from religious conviction. -- Blaise Pascal (1623-1662)
New That is an external ID
The internal system should still uniquely identify them internally. The customer's number is just an attribute that is passed on. One should not rely on it for providing unique tracking/labling.

I suppose some companies don't track them as self-standing entities, just simply associate them with the sales order or a new in-progress invoice.
________________
oop.ismad.com
Expand Edited by tablizer Nov. 10, 2001, 08:25:25 PM EST
New Yep. You got it.
Alex

Men never do evil so completely and cheerfully as when they do it from religious conviction. -- Blaise Pascal (1623-1662)
New Then why not simply keep it an attrib instd of object?
________________
oop.ismad.com
New Excellent place for code reuse.
I have two objects, both of which are almost identical, except for that one of them has to use a unique id, where the other one doesn't. Inherit, override, and reuse most of the code.

I just saved myself some time recoding the exact same thing twice.
"He who fights with monsters might take care lest he thereby become a monster. And if you gaze for long into an abyss, the abyss gazes also into you." - Friedrich Nietzsche
New Good comments
I always start my member var names with an underscore so I don't have to keep going this.name = name; instead I say _name = name and I can tell at a glance whats an ivar and whats a param or temp.

I would just add here that "this." makes it quite explicit where the variable is coming from, which is a good thing. I tend to do that, or prepend member variables with 'm' or the like. Whatever makes sense to you, but the key is to make the member variables stand out in the code explicitly. I don't tend to like underscores because they're easy to miss (especially for me because of my eye problems).

Regards,

-scott anderson
     Anybody want to do a Java code review? - (inthane-chan) - (13)
         Of the top of my head.... - (ChrisR)
         Re: Anybody want to do a Java code review? - (bluke) - (1)
             This is exactly why I need the review. - (inthane-chan)
         Couple comments - (tuberculosis) - (8)
             Thanks - (inthane-chan) - (6)
                 questions - (tablizer) - (5)
                     Purchase Order ID is specified by the purchaser, i.e... - (a6l6e6x) - (4)
                         That is an external ID - (tablizer) - (3)
                             Yep. You got it. -NT - (a6l6e6x) - (2)
                                 Then why not simply keep it an attrib instd of object? -NT - (tablizer) - (1)
                                     Excellent place for code reuse. - (inthane-chan)
             Good comments - (admin)
         Re: Anybody want to do a Java code review? - (bri)

IBM is good at two things:
  1. Shooting itself in the foot, and
  2. Reloading.

218 ms