On Wed, 2008-09-17 at 09:20 -0400, Ryan McKinley wrote:
> On Sep 17, 2008, at 5:25 AM, Thorsten Scherler wrote:
> 
> > On Tue, 2008-09-16 at 14:57 -0400, Ryan McKinley wrote:
> >> Looking over the API some more...
> >>
> >> Is the (can the) Queue be Task agnostic?  That is, can the Queue
> >> accept any task even if invalid?
> >
> > We do not have any criteria till now to decide whether a task is valid
> > or not.
> >
> >> Can we move all acceptX() methods?
> >
> > Not sure what you mean, which acceptX method?
> >
> 
> right now, QueueBean has:  acceptSize(int i)  and acceptDepth(int i)
> 
> Since the merge() function does not actually validate (nor do i think  
> it should), I think depth validation etc should go elsewhere.


The simple queue impl. does:

public void merge(Task[] filterLinks) {
    LinkedList<Task> list = new LinkedList<Task>();
    if (null != getToDoTasks()) {
      log.debug("getToDoTasks().length " +getToDoTasks().length);
      for (int i = 0; i < getToDoTasks().length; i++) {
        Task task = getToDoTasks()[i];
// here we validate that we do not add too many tasks nor bounce the
depth
        if (null != task && acceptSize(i + allTasks.size())
            && acceptDepth(task.getDepth())) {
          list.add(task);
        }
      }
    }
    for (int i = 0; i < filterLinks.length; i++) {
      Task task = filterLinks[i];
// here we validate that we do not add too many tasks nor bounce the
depth nor whether a task with the same id is already contained
      if (null != task && acceptSize(i + allTasks.size())
          && !allTasks.containsKey(task.getId())
          && acceptDepth(task.getDepth())) {
        log.debug("allTasks.size() "+allTasks.size());
        allTasks.put(task.getId(), task);
        list.add(task);
      }
    }
    setToDoTasks(list.toArray(new Task[list.size()]));
  }

> >>
> >> I like this because it pushes validation logic elsewhere and makes  
> >> the
> >> Queue very general. 

The validation has to be done in the implementation this allows to
implement your own validation when you need it. 

> >> perhaps we can add an isFull() method to Queue
> >
> > This will return if the queue size = maxSize?
> >
> 
> yes, this way whoever is doing validation can ask if this will blow up  
> first.

Something like:

Index: src/core/java/org/apache/droids/queue/Simple.java
...
@@ -89,7 +89,7 @@
     for (int i = 0; i < filterLinks.length; i++) {
       Task task = filterLinks[i];
       
-      if (null != task && acceptSize(i + allTasks.size())
+      if (null != task && isFull(i)
           && !allTasks.containsKey(task.getId())
           && acceptDepth(task.getDepth())) {
         log.debug("allTasks.size() "+allTasks.size());
@@ -100,6 +100,10 @@
     setToDoTasks(list.toArray(new Task[list.size()]));
   }
 
+  private boolean isFull(int i) {
+    return acceptSize(i + allTasks.size());
+  }

then we should call it willBeFull(i) but as you can see, it is a helper
method for acceptSize.

> 
> hymm, maybe we do need acceptSize( x ) so that we can ask if a  
> Collection of tasks will fit?

Definitely or we can use willBeFull(i) instead. Or rename it to
acceptIncrease(i) and merge them.

> 
> Either way, I think the "do the Tasks fit?" question should be asked  
> to the Queue interface rather then the sub-implementation.

I agree for the common use case. IMO the acceptIncrease(i) express it
the best for size validation. However if you think much more general
like PM (project management) wise the validation of a task is a whole
other playground and acceptIncrease(int i) will not make much sense.
Here an accept(Task task) would be the one. 

Having said this we may want to extend the Queue interface with a
CrawlQueue interface and when the use case comes a PMQueue interface.
Here we are free to specify what we await. Actually the DelayWorker is
born like this.

We have to try keeping the interface as open as possible to not close us
for a rare use case. Meeting droids slogan "Take what you need, do what
you want, impossible is nothing." needs slim really general interfaces
and specialist use case based once extending them. 

Does that makes sense?

salu2

> 
> ryan
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
> 
-- 
Thorsten Scherler                                 thorsten.at.apache.org
Open Source Java                      consulting, training and solutions


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to