[URLyBird]Why not come in and debate on my code of find?

[Zhixiong Pan]

Hi ranchers,
I recently completed the find method in Data. I show the whole code here, hoping your advices and rectifications .

    public int[] find(String[] criteria){
  HashMap result = new HashMap();
  Vector satisfiedRecNo = new Vector();
  int[] foundRecs;
  for(int recNo = 1; ; recNo++){
  try{
  String[] record = read(recNo);
  result.put(new Integer(recNo),record);
  satisfiedRecNo.add(new Integer(recNo));
  }
  catch(RecordNotFoundException e){
  if(e.description.equals("deletedRecord"))
    continue;
  if(e.description.equals("overflowed"))
    break;
  }
  }
  for(int i = 0; i < 7; i++){
  if(criteria[i] == null)
    continue;
  Iterator ite = satisfiedRecNo.iterator();
  Vector temp = new Vector();
  while(ite.hasNext()){
  Integer key = (Integer)ite.next();
  String s = ((String[])result.get(key))[i];
  if(s.indexOf(criteria[i]) == 0)
    temp.add(key);
  }
  satisfiedRecNo = temp;
  }
  foundRecs = new int[satisfiedRecNo.size()];
  for(int i = 0; i < satisfiedRecNo.size(); i++){
  foundRecs[i] = ((Integer)satisfiedRecNo.elementAt(i)).intValue();
  }
  return foundRecs;
  }  

Some comments are as following:
1) my read() synchronize RAF and throw RecordNotFoundException with a String parameter "deletedRecord" in case of a deleted record and throw RecordNotFoundException with a parameter "overflowed" if the RAF's pointer out of the length of RAF.
2) i dont synchronize RAF in this method, cause the data will also be out of date even if i do such sychronizing. So i use a Collection to store the result of read().
Debate on improvement of arithmetic, brevity of coding, idea of designing are welcome and expected here.

[Petr Hejl]

You should never use exception description in program!!!
1) exception type -> for program
2) exception stacktrace -> for programmer
3) exception message -> for user

Better should be to made descendants of RecordNotFoundException.

Oh, now I see description is parameter, but generaly this so called "enumerated exceptions" are bad idea. I would prefer something like
for(int i = 0; i < getRawRecordCount(); i++). Yes just after cycle ends the new record can be added, but this can happen in your code too. There is no chance (if you do not lock whole db for find) to avoid this - it is a real world
[ April 05, 2006: Message edited by: Petr Hejl ]

[Richard Green]

I would rather do it like this

public int[] find(String[] criteria){
        int i = 0;
        Set<Integer> lhs = new LinkedHashSet<Integer>();
        while (true) {
            try {
                if (matchesCriteria(criteria[],readRecord(i))) {
                    lhs.add(i);
                }
                i++;
            } catch (RecordNotFoundException re) {
                break;
            }
        }
        Integer[] x = lhs.toArray(new Integer[lhs.size()]);
        //... convert the Integer[] to int[] and return;
        return ret;
}

[ April 05, 2006: Message edited by: Mike Corleone ]

[Petr Hejl]

But if record number 0 would be empty record (deleted flag) you wouldnt find anything - worst case.

[Jason Moors]

You also need to think of ways to optimise the find method. If all the fields are null, i.e. there is no search criteria, do you need to compare the field values?

Jason

[Oricio Ocle]

Personally i use to avoid the usage of public method calls within other instance methods if those throw their own exceptions.
In your case i would define a private method like:

private String[] getFileRecord(long recNo) {
    ...
}

for being used inside other instance methods like find and read:

public String[] read(long recNo) throws RecordNotFoundException {
    ...
    String[] recordString = getFileRecord(recNo);
    if(...){
        throw new RecordNotFoundException(...);
    }
    ...
}

That is
'keep your exceptions for your clients'.

Regards
Ori
[ April 05, 2006: Message edited by: Oricio Ocle ]

[Zhixiong Pan]

To Jason,

If all the fields are null, i.e. there is no search criteria, do you need to compare the field values?

In case of all elements in criteria are null, my code will "continue" server times and then i (in for(int i = 0; i < 7; i++) ) will be 7, so no compare happened.

To Oricio,

In my opinion, adding new methods to Data is not a good choice, so I didnot yet use private getFileRecord().

In your case, every time you read one record, you will have to store all records into recordString first. Isn't it a waste of time?


To all,

Should I not use RecordNotFoundException's description in DB Layre? If not, could you give me an approach to get the status of deleted record and pointer at the end of RAF in the find method.

[Oricio Ocle]

Hi Zhixiong!
Avoid inner exceptions is an extended good practice. Moreover if the exceptions are thrown inside a loop, and moreover if these exceptions are expected to occur frequently, as is your case.
You can google something like "exception cost" java and find lots of explanations.
Any way if you only are intested in performance, you can try something like this and extract your own conclusions:

public class Test {
 
private  long maxIterations;
private  double exProbability;

public Test(long maxIterations,double exProbability){
this.maxIterations = maxIterations;
this.exProbability = exProbability;
}
private String[] read(int index) throws Exception{
if(index%Math.round(1/exProbability) == 0){
throw new Exception("");
}
return new String[7];
}
 
private String[] getFieldRecord(int index){
if(index%Math.round(1/exProbability) == 0){
return null;
}
return new String[7];
}
 

public double readATest(){
long initMillis = System.currentTimeMillis(); 
for(int i = 1 ;  i < maxIterations ; i ++){
try{  
String[] record = read(i);
} catch(Exception e) {  
continue;
}
}
return System.currentTimeMillis() - initMillis;
}
 
public double readBTest(){
long initMillis = System.currentTimeMillis();
for(int i = 1 ;  i < maxIterations ; i ++){
String[] record = getFieldRecord(i);
if(record == null) continue;
}
return System.currentTimeMillis() - initMillis;
}
 

public static void main(String[] args){
Test test = new Test(1000000,0.5);
System.out.println(
"Elapsed :\n" +
"\ttestA (throwing exceptions): " + test.readATest() + " ms\n" +
"\ttestB (avoiding exceptions): " + test.readBTest() + " ms");
}
}

Regards
[ April 06, 2006: Message edited by: Oricio Ocle ]

[Zhixiong Pan]

Hi Oricio,

After run your code, I can clearly see that Exception throwing is a slow procedure. Thanks for your good code.

So I did some modification to my find.
1) In Data I create a Vector variable deletedRecordsNo store deleted records' number.
2) add

  deletedRecordsNo.add(new Integer(recNo));  

into delete method
3) remove

  if(e.description.equals("deletedRecord"))    continue;  

and add

  if(deletedRecordsNo.contains(new Integer(recNo)))
    continue;  

before 'try'.
But a disadvantage can be easily seen that if server crash, then the deleted records can not be recognized.Does such a disadvantage matter and influence the score?

[Oricio Ocle]

Hello Zhixiong,

In Data I create a Vector variable deletedRecordsNo store deleted records'

.
IMO, that procedure adds complexity and potential problems for a doubtful benefit.
Other case is implementing a full database cache, that obviously would provide a performance improvement, since, for example, you are reducing disk access.
You can seach for "cache" in this forum.
Regards