wood burning stoves 2.0*
The moose likes Developer Certification (SCJD/OCMJD) and the fly likes about criteriaFind Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Murach's Java Servlets and JSP this week in the Servlets forum!
JavaRanch » Java Forums » Certification » Developer Certification (SCJD/OCMJD)
Bookmark "about criteriaFind" Watch "about criteriaFind" New topic
Author

about criteriaFind

Bigwood Liu
Ranch Hand

Joined: Feb 26, 2003
Posts: 240
hi,
My criteriaFind is as following:

Regards,
Damu
[ June 07, 2003: Message edited by: damu liu ]
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11404
    
  81

Hi Damu,
Sorry, you have to code in Java - psuedocode is not allowed
Seriously though ....
It appears that if a column name is not found, then the bad column name is ignored, but the search continues with the remaining column names that are valid.
Do you have the instruction:
In the event of an invalid field name being provided as part of the criteria the behavior of this method is the same as if no records matched correctly specified criteria.

If so, I think you may end up with records that appear to match the invalid criteria.
Otherwise it appears that this will work.
Just some things you might want to think about:
Have you considered using a Map instead of your two lists?
Have you considered using StringTokenizer instead of the substring()/indexof() methods? Or the regular expression package (I dont recommend it for this assignment, but you might want to consider it).
Regards, Andrew


The Sun Certified Java Developer Exam with J2SE 5: paper version from Amazon, PDF from Apress, Online reference: Books 24x7 Personal blog
Bigwood Liu
Ranch Hand

Joined: Feb 26, 2003
Posts: 240
Hi,
thank you for reply Andrew,

Have you considered using a Map instead of your two lists?

I had thought to put the indexes and the values into a map, but when I go through all the records and do the comparing, I don't know how to take them out from the map one by one without knowing what is in the map.

Have you considered using StringTokenizer instead of the substring()/indexof()

I had read a lot of articles here about using StringTokenizer, but I think substring()/indexof() can parse the string easly, while StringTokenizer have to deal with diffrent delimiter:"=","'".
My code is as following:

Regards,
Damu
Bigwood Liu
Ranch Hand

Joined: Feb 26, 2003
Posts: 240
hi Andrew,
Do you think this method is to big, and I should divide it into several little methods?
Regards,
Damu
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11404
    
  81

Hi Damu,
I had thought to put the indexes and the values into a map, but when I go through all the records and do the comparing, I don't know how to take them out from the map one by one without knowing what is in the map.

It is fairly easy, you create an iterator over either the key or the value, and extract the other item:

It could be argued that the extra work is not worth the effort. Your decision.
I had a separate method which converted the string supplied by the user into a map which was used by the criteriaFind method. So in my case having the data in a single map which was returned by the conversion method made my code easier to use.
I am not trying to convince you of one way over the other - your way seems fine to me - I am just giving you things to think about
And now for some more things to think about (to stop you from getting bored )
You have catered for the criteria being enclosed in quotation marks, but you have not catered for the column name being enclosed in quotation marks.
I think your quotation handling could cause problems if the column name contains a quotation mark (e.g. Bill'sAirline) but the entire string is not enclosed in quotation marks - does this concern you?
I think if record 'x' matches the criteria, and record 'x+1' is deleted, then the deleted record will be added to your criteriaList ArrayList.
I am not sure whether you should be reusing CriteriaList for two different purposes - I think it would be better (although slightly wasteful) to have a separate ArrayList for this function.
Think about the scope of your variables - I think you could reduce scope on a lot of them.
Why aren't you using the toArray() method instead of

Do you think this method is to big, and I should divide it into several little methods?

I dont think it is too big in terms of number of lines, however I think that clarity might be improved by breaking it into functional areas. At the moment, your documentation might read "converts user criteria from string format to lists, AND then finds matching records." Anywhere that an AND appears in documentation is a possible indication that you are violating the One responsibility rule. But this is subjective - you have to decide whether refactoring will make it more confusing.
Regards, Andrew
[ June 07, 2003: Message edited by: Andrew Monkhouse ]
Bigwood Liu
Ranch Hand

Joined: Feb 26, 2003
Posts: 240
Hi Andrew,
Thank you very much.
Your answer is great valuable to me. I will rethink this method.
Best Regards,
Damu
[ June 08, 2003: Message edited by: damu liu ]
Bigwood Liu
Ranch Hand

Joined: Feb 26, 2003
Posts: 240
when I do toArray():

I get a runtime ClassCastException .
in addition, I add a row of code as this to avoid adding a 'x+1' deleted record into the criteriaList:

[ June 08, 2003: Message edited by: damu liu ]
[ June 08, 2003: Message edited by: damu liu ]
Mark Spritzler
ranger
Sheriff

Joined: Feb 05, 2001
Posts: 17249
    
    6

while (tokenizer.hasMoreTokens()) { criteriaList.add(tokenizer.nextToken().trim()); }
To me that looks like you are just putting the Strings into the ArrayList. Whereas, you are trying to cast it to a DataInfo[]. String[] <> DataInfo[].
I hope that is it.
I would argue that your method is too big, it can be much more readible and manageable if it is broken into multiple methods, each with the one responsibility rule.
Mark


Perfect World Programming, LLC - Two Laptop Bag - Tube Organizer
How to Ask Questions the Smart Way FAQ
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11404
    
  81

Sorry Mark, no prizes for you today You got caught by the fact that Damu is using the criteriaList variable for two different functions, holding two different types of objects.
Damu: this is why I recommend that you do not reuse variables in this manner ... it is too easy for someone reading your code to be confused.
The following code worked for me:
Sorry I cannot currently remember why the normal toArray() without any parameters does not work, but specifying the type of data as a parameter does work.
Regards, Andrew
Bigwood Liu
Ranch Hand

Joined: Feb 26, 2003
Posts: 240
Hi ,
Thank you both.
I am trying to divide this method to make it more readable.
it is too easy for someone reading your code to be confused.

You are right, Andrew! I already made Mark confused .I will correct it.

this does work!
Thank you once more, Andrew
Regards,
Damu
Bigwood Liu
Ranch Hand

Joined: Feb 26, 2003
Posts: 240
Hi,
this is my new version criteriaFind:
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11404
    
  81

Hi Damu
You call iterator.next() twice in this bit of code:

This is almost certainly wrong.
Something else to think about ... do you prefer this line of code: or these two lines:
It can certainly be argued either way - it is up to you how you want to code this.
I didnt bother casting the desiredValue back into a String - I left it to the String equals() method to handle that. Once again, you could argue for or against the explicit cast.
I presume you are going to do something with that Exception handling routine?
Regards, Andrew
S. Ganapathy
Ranch Hand

Joined: Mar 26, 2003
Posts: 194
I really suggest Damu not to use so huge code in methods. Try to refracter your code damu.
I too implemented search functionality. Please review my code too.
Delimiter I used is either ',' OR ', '
Usage: name='someName', location='someLocation', etc..

Please note that the above code is not final one. This only gives an idea of search functionality
Please review my code too and let me know your comments.
Thanks in advance.
Regards,
Ganapathy.
[ June 08, 2003: Message edited by: S. Ganapathy ]
Bigwood Liu
Ranch Hand

Joined: Feb 26, 2003
Posts: 240
Hi Andrew,
thank you!
Is this ok?
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11404
    
  81

Hi Ganapathy,
Wow - are you trying to make me engage my brain by using regex?
I assume that where you put the comment:
You are converting column names to column indexes. Is this correct?
Assuming the above, your regex allows the column name to have up to two words seperated by spaces. This will fail if the column names have three words. Could this be a potential problem? (I dont have your assignment so I dont know)
Your regex will fail if either the column name or the criteria to match contains a single quote (e.g. "O'briensContractors") - does this concern you?
Some general comments ... I strongly recommend against swallowing exceptions like this:
If you want to ignore the exception while developing and come back to it later, then put a comment in there to remind yourself - preferably using a notation that makes it easy to find later. Personally I do this by using my initials in the comment (and I dont use them elsewhere), so I can always search for the magic string ADM to see where I have something left to fix:
This also helps when you put code up for review - the reviewer then knows what your intention is.
Your findByCriteria method works on an array of activeRecords. However there is nothing to stop additional records being added, or existing records being deleted while you are inside this method. Is this OK for you? Are you commenting this?
As with my comment to Damu - why are you iterating through the returnList to put the items it contains into an array, when the toArray() method will do this for you?
In you match() method - nice use of &= (last time I did that, my team spent 15 minutes reviewing 200 lines of code, and 45 minutes reviewing my use of &= ) but I wonder if it is worthwhile? If you are willing to break the concept of having only one "return" statement in a method, then you can return false the moment that you know that the data does not match the criteria, and return true if you get through the loop otherwise. With such a short block of code and appropriate comments, you can get away with this. However if you wish to stay "pure" then this is fine.
Regards, Andrew
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11404
    
  81

Hi Damu,
Your modification looks fine.
Regards, Andrew
S. Ganapathy
Ranch Hand

Joined: Mar 26, 2003
Posts: 194
Hi Andrew,
I am not swallowing exceptions. I need to handle them. I will provide comments as you suggests, and later I will handle them.
I have not tested regular expression properly. I will update the expression, if I am able to makeout one, or else I use StringTokenizer to split the name value pairs. Thanks for pointingout the parseing bug.
I am converting column names to column indexes. You are right.

Your findByCriteria method works on an array of activeRecords. However there is nothing to stop additional records being added, or existing records being deleted while you are inside this method. Is this OK for you? Are you commenting this?

I am making suitable assumptions like there are no new records are added while searching. But if the records are deleted, after collecting list of active records, while reading the record with record number, it will throw RNF exception. Only adding records is a problem but I am writing a note in choices.txt.
So, is this match() OK?
There is no much performance variation because array has only 6 elements, and readability is good in previous case, as there is only one return statement.

I am sure, I have to look at parseCriteria() seriously.

As with my comment to Damu - why are you iterating through the returnList to put the items it contains into an array, when the toArray() method will do this for you?

I can only add Object types in Collection objects.

That is the reason, I am using Iterator.
Your comments are most welcome.
Regards,
Ganapathy.
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11404
    
  81

Hi Ganapathy
But if the records are deleted, after collecting list of active records, while reading the record with record number, it will throw RNF exception.

OK - I didnt see that in your code.
I wonder if throwing an exception is a good thing or not? I think this is one case where you could just handle the deleted record as you go along.
Your new match looks OK to me. The old one did too, I just like to play devil's advocate As for the performance, it could make a difference if you had to itterate over 1000 records multiplied by all 6 elements, compared to the new code which only has to go over the elements until the point where it knows whether the record matches or not. However perfomance is not an issue in this assignment, so I wouldnt let that worry you.
I missed that you were converting from an array of Long to an array of long. Ooops, my bad.
Regards, Andrew
S. Ganapathy
Ranch Hand

Joined: Mar 26, 2003
Posts: 194
Hi Andrew,
I have to swallow RNF Exception, while collecting elements. So I have to be very careful to handle and swallow exceptions. This is a big exercis to me.
Thanks once again Andrew for your express replies.
Regards,
Ganapathy.
Vitaly Zhuravlyov
Greenhorn

Joined: Apr 11, 2003
Posts: 16
Hi Andrew and Ganapathy,
Originally posted by Andrew Monkhouse:
In you match() method - nice use of &= (last time I did that, my team spent 15 minutes reviewing 200 lines of code, and 45 minutes reviewing my use of &= ) but I wonder if it is worthwhile?

It certainly is not worthwhile

is always found and

is always false
Vitaly
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11404
    
  81

Hi Vitaly
Ooops, I was busy being impressed that someone else used &= and ignored the fact that it was operating on a boolean. Thanks for spotting it and bringing it to our attention.
Just had to go back and check the code that I wrote a while back that also used bitwise assignment operators. Fortuanately my usage was valid however it was written in a C program, so I wont post it here.
Regards, Andrew
Vitaly Zhuravlyov
Greenhorn

Joined: Apr 11, 2003
Posts: 16
You are welcome Andrew!
It's not often that people reply to my messages so promptly Because of the time zones
Vitaly
S. Ganapathy
Ranch Hand

Joined: Mar 26, 2003
Posts: 194
Hi Andrew,
I again started concentrating on criteriaFind. I modified my regular expression. Please go through te regex once again.
Usage: name="someName", location="someLocation", etc..

I am assuming that the double quote never occur in the value of name-value pair. I am documenting this.
I am not interested to use StringTokenizer, as I need to use twice go split name-value pairs. Regular expression will make the code simple.
Please give your openions and comments.
Regards,
Ganapathy.
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11404
    
  81

Hi Ganapathy,
The only difference is that you are now using double quotes instead of single quotes. This solves the issue I raised: Your regex will fail if either the column name or the criteria to match contains a single quote (e.g. "O'briensContractors") - does this concern you?
So, this should be fine. I assume you are also documenting the other issue I raised.
By the way, do you have a requirement that the value to be searched for should be between quotation marks?
Regular expression will make the code simple.


I like regular expressions, and use them a lot. And I like the fact that some people here are using them in the assignment. But simple? I would argue that a junior programmer could read and understand larger code that uses StringTokenizer faster than they could read and understand regex. I think the same statment could even be applied to most non junior programmers. But keep using regex if you are comfortable with it.

Regards, Andrew
S. Ganapathy
Ranch Hand

Joined: Mar 26, 2003
Posts: 194
Hi Andrew,
The only difference is that you are now using double quotes instead of single quotes. This solves the issue I raised: Your regex will fail if either the column name or the criteria to match contains a single quote (e.g. "O'briensContractors") - does this concern you?

I tried using single quotes, and it did not fail. Earlier, I forgot to consider using single quote. Now it is fine. e.g. name="Monster's Website" will work fine.
I tried StringTokenizer, and String.split(regex), and I found using regular expression is good.
Ofcourse, for junior programmers, regular expression is difficult. But they need not understand regular expression extensively. This will be documented in the method. So there won't be any problem.
Regards,
Ganapathy.
Eugene Sun
Greenhorn

Joined: Aug 09, 2003
Posts: 17
Hi all,
A couple of questions here about FBN,
1. Has anyone used StringTokenizer to parse the criteria String instead of the java.util.regx package and passed the exam without losing points on criteriaFind method implementation ?
2. I have both the "Origin City" and "Destination City" as input TextFields, has anyone implemented them this way and passed without being penalized, or do I have to implement a ComboBox with a selection of cities for these two input fields ?

Thanks,
Eugene
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11404
    
  81

Hi Eugene
1. Has anyone used StringTokenizer to parse the criteria String instead of the java.util.regx package and passed the exam without losing points on criteriaFind method implementation ?

Up until recently it was unclear whether JDK 1.4 could even be used for Fly By Night Services. Sun eventually declared that it was allowable though. It has only been since then that people have even started looking at regex.
Which is a long way of saying that there are lots of people who have passed using StringTokenizer.
2. I have both the "Origin City" and "Destination City" as input TextFields, has anyone implemented them this way and passed without being penalized, or do I have to implement a ComboBox with a selection of cities for these two input fields ?

I don't know if anyone has passed that way.
I do know that at least one person failed, and received the comment from the examiner that they had not used JComboBoxes (caused quite a bit of concern at the time, and I think that person later passed on appeal).
Personally I only used JComboBoxes and got 100% for my GUI, so I guess they liked it
Regards, Andrew
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: about criteriaFind
 
Similar Threads
search algorithm: clarity and efficiency
Modifying DataInfo
criteriaFind review please...
Question about the use of JComboBox
Comments on final server design