My If Statements Aren't Catching What They Should

I am trying to implement a dating agency system for my university project. The section I have been allocated to do is to search for a partner that matches a user. We have preset data so that it allows us to test the system is working, however it doesn't seem to be for me.

The very first check that I make is that the sexual orientation of a user and their gender matches up with that of the record being checked, but in one case this check doesn't seem to be working.

One of the preset users is a heterosexual female, another is a bisexual female. These two users do not match because the first user does not want a partner of the same sex, however the system is matching these two up.

Could you please look at my code and help me identify where the problem is? I have been spending about the last hour and a half trying to work out why it isn't working but I can't figure it out. I have traced through the method I use, using these two users as my data and it seems to me as if it should be working.

The following is the method used to search for a matching partner, as I said, it seems to be the very first if statement that the problem is with because in the case of these 2 users, it is falling at the first hurdle.

The great thing about Object Oriented code is that it can make small, simple problems look like large, complex ones

09 F9 11 02
9D 74 E3 5B
D8 41 56 C5
63 56 88 C0
Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems. -- Jamie ZawinskiDetavil - the devil is in the detail, allegedly, and I use the term advisedly, allegedly ... oh, no, wait I did ...BIT COINS ANYONE

Thanks, I will try printing to the console and see what it comes up with.

Unfortunately we can't make a database for this because it's a software development module and we have been told we have to create it using swing. It would make life so much easier to do it in a database but unfortunately life isn't always easy

Chat Server Project & Tutorial | WiFi-remote-control sailboat (building) | Joke Thread
“Rational thinkers deplore the excesses of democracy; it abuses the individual and elevates the mob. The death of Socrates was its finest fruit.”
Use XXX in a comment to flag something that is bogus but works. Use FIXME to flag something that is bogus and broken. Use TODO to leave yourself reminders. Calling a program finished before all these points are checked off is lazy.
-Partial Credit: Sun

If I ask you to redescribe your problem, it's because when you describe issues in detail, you often get a *click* and you suddenly know the solutions.
Ches Koblents

try the pen and paper method. input some data into your program and c what it gives. then do the same thing but manually. print the code out and use the same data to go through your code. you'll prolly find the problem.

best thing would be to rewrite your code in a less complex form though by using operators (|| && )

Besides the fact that your code does need some serious refactoring, the logic of compareSexualOrientations, although not very efficient, works.
Did you print the gender and orientations in the console? My guess is that you're not comparing the two persons that you think you are.

To refactor your compareSexualOrientations method you first describe it's logic in plain English:
If two persons are of the same gender and both are not of heterosexual orientation, they match. If two persons are of different gender and both are not of homosexual orientation they match.
We could translate this to Java lik ethis:[hl=java]//It's always good to use constants in situations like this
static final String HETEROSEXUAL = "Heterosexual";
static final String HOMOSEXUAL = "Homosexual";
static final String BISEXUAL = "Bisexual";

public boolean compareSexualOrientations(int pCount) {
boolean match = false;
//Some will probably object to the following style.
//In cases where performance is not a big issue and
//the possible values allow it, I think it is an easy
//way to avoid endless if statements.
String orientations = users[currentUser].getSexualOrientation()+users[pCount].getSexualOrientation();

if ((users[currentUser].getGender() == users[pCount].getGender() &&
orientations.indexOf(HETEROSEXUAL) == -1) ||
(users[currentUser].getGender() != users[pCount].getGender() &&
orientations.indexOf(HOMOSEXUAL) == -1)) {
match = true;
}
//Single exit point, always nice
return match;
}
[/hl]Another (more preferable IMO) way would be to make the matching process part of the User, Person, or WhateverYouCalledIt class that is in the user array. The same logic as above still applies though.

Comments on this post

jzd agrees

Last edited by wsa1971; March 27th, 2007 at 06:37 AM.
Reason: Had some time to kill

A common mistake people make when trying to design something completely foolproof is to underestimate the ingenuity of complete fools.Douglas Adams

Unfortunately we can't make a database for this because it's a software development module and we have been told we have to create it using swing. It would make life so much easier to do it in a database but unfortunately life isn't always easy

That makes no sense. Swing is just a gui development library. The application can be written to use swing for display and still draw information from a database through JDBC. Are you sure you understood the requirements correctly?

Comments on this post

Axweildr agrees

Dear God. What is it like in your funny little brains? It must be so boring.

Another (more preferable IMO) way would be to make the matching process part of the User, Person, or WhateverYouCalledIt class that is in the user array.

Indeed. In order to properly leverage encapsulation, one should let the User/Person/WhateverYouCalledIt object tell you whether or not it's compatible with another user rather than evaluating that logic externally. For example, the User/Person/WhateverYouCalledIt should implement a method along the lines of the following:

The logic of whether or not one person is compatible with another should reside in the Person class. Think about it: if I want to know who you're compatible with, I should ask *you*, not someone else, right?

Using good object-oriented programming practices will help make your code less brittle.

@Yawmark: You do realise that your (otherwise excellent) example will match a lesbian female with a gay man (and a bisexual male to a heterosexual male etc...), right?

Originally Posted by Yawmark

The logic of whether or not one person is compatible with another should reside in the Person class. Think about it: if I want to know who you're compatible with, I should ask *you*, not someone else, right?

You should, but in the real world a lot of people do exactly the opposite.

Last edited by wsa1971; March 27th, 2007 at 07:47 AM.

A common mistake people make when trying to design something completely foolproof is to underestimate the ingenuity of complete fools.Douglas Adams

Okay, here's some slightly less embarassing business logic, with a *big* caveat: More complex "compatibility" requirements will -- obviously -- require a more complex algorithm, which should likely be encapsulated in its own class; e.g., a CompatibilityStrategy or some such thing.

Hmm. I would just use 0, 1, and 2 for preference. 0 = guys, 1 = both, 2 = girls. I think you overcomplicated it.

Chat Server Project & Tutorial | WiFi-remote-control sailboat (building) | Joke Thread
“Rational thinkers deplore the excesses of democracy; it abuses the individual and elevates the mob. The death of Socrates was its finest fruit.”
Use XXX in a comment to flag something that is bogus but works. Use FIXME to flag something that is bogus and broken. Use TODO to leave yourself reminders. Calling a program finished before all these points are checked off is lazy.
-Partial Credit: Sun

If I ask you to redescribe your problem, it's because when you describe issues in detail, you often get a *click* and you suddenly know the solutions.
Ches Koblents

Hmm. I would just use 0, 1, and 2 for preference. 0 = guys, 1 = both, 2 = girls. I think you overcomplicated it.

So we're still on the business logic, eh? I think using a simple preference oversimplifies the situation. A bisexual female may prefer a heterosexual female, but that preference is not likely to be reciprocated (this problem is mentioned in the OP). If you're hoping to determine compatibility, you'll need to look at both sides of the aisle, so to speak. I found that to be pretty complicated, actually. If there's a way to evaluate compatibility based solely on the preference of one particular side of the matchup, I don't see it.

But all this misses the point. The gist of my posts (in support of wsa1971's design suggestion) is that this logic belongs within the object that contains the data rather than asking an object for its data. That's the whole principle of encapsulation and data hiding. By encapsulating the logic this way, the implementation details are irrelevant to callers. You can use enums, strings, int values, or whatever to determine compatibility, and client code never has to change. Exposing int values for compatability calculations is a very brittle, highly-coupling solution.