This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 138264 - DataView.getExceptions() should return list of Exceptions
Summary: DataView.getExceptions() should return list of Exceptions
Status: RESOLVED FIXED
Alias: None
Product: db
Classification: Unclassified
Component: Show Data (show other bugs)
Version: 6.x
Hardware: Macintosh All
: P1 blocker (vote)
Assignee: _ ahimanikya
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-06-25 18:43 UTC by David Vancouvering
Modified: 2008-06-27 08:32 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Vancouvering 2008-06-25 18:43:19 UTC
Please change DataView.getExceptions() to be like this:

Iterator<Throwable> getExceptions()

Thanks.
Comment 1 _ ahimanikya 2008-06-25 21:30:15 UTC
Fixed
Comment 2 Andrei Badea 2008-06-26 09:19:49 UTC
Ahi: when you fix an issue please mention the changeset.

David: the result can't be an Iterator, because you can only iterate it once. I suggest a Collection instead.
Comment 3 Andrei Badea 2008-06-26 09:20:11 UTC
Reopening.
Comment 4 _ ahimanikya 2008-06-26 09:30:00 UTC
This was already checked in before David created the issue as part of API change request...

Anyway, why do you need an random access to these exceptions? You can always get the iterator back from dataview, if
required again.
Comment 5 Andrei Badea 2008-06-26 12:21:18 UTC
I don't need random access, that's why I proposed Collection, not List.

It isn't true that you can always get the iterator from the DataView instance, for example when you don't have the
DataView instance available because you passed the iterator to a method and for some reason you don't want to also pass
DataView to that method.

Also, many API methods in the Collections API take a Collection as parameter, not an interator.
Comment 6 _ ahimanikya 2008-06-26 12:40:19 UTC
But why do you want to pass around this Collection of Exceptions? SQLEditor will get hold of these exceptions through
SQLExecutionResult, in such case you don't need to pass DataView instances around. I will let you guys decide, but I
don't see the need. 
Comment 7 Andrei Badea 2008-06-26 13:25:16 UTC
I probably wouldn't. But when you create an API, you need to think about more use cases than the one you are just
solving, and you should try to make your API flexible enough.

Another example: imagine you need to look at the list of exceptions in a debugging session. If you put the iterator in a
field or variable and display it in the Watches window, you can only display it once (because you can only iterate once).
Comment 8 David Vancouvering 2008-06-26 14:29:28 UTC
Thanks, Andrei, I knew there was something bothering me about Iterators but I couldn't put my finger on it.  I think
Andrei has nailed it.  

One particular case that I already encountered: you can't use Java 5 looping constructs on an Iterator:

Iterator<Throwable> exceptions = view.getExceptions();
for (Throwable t : exceptions) {
  logException(t);
}

doesn't work, for example.
Comment 9 _ ahimanikya 2008-06-26 14:44:37 UTC
I am not sure what makes you think I did not think of more use cases here. I just have a disagreement with what you are
saying, that does not mean I have not thought enough about it. Also, I am not sure why you think Iterators are not
flexible. Iterators are designed to be used for APIs to provide abstraction to the underlaying collections. In the same
time, I do understand that everyone has different prospective. Lets just agree to disagree here.

BTW, In a debugging session, you can always watch the DataView instance to get access to underlaying collections, I
don't think this should be a criteria for API design. Data integrity is always a big concern and Iterators are just
great in a handshaking mode between components/modules. I felt in the current situation a Iterator is suffice.

If you guys want I am willing to change it.
Comment 10 _ ahimanikya 2008-06-26 14:52:47 UTC
This is yet another wrong reason for not using Iterator here. You can always use the following. To me, its just a matter
of style.

Iterator<Throwable> exceptions = view.getExceptions();
while(exceptions.hasNext();) {
    logException(exceptions.next());
}

OR

for (Iterator<Throwable> exceptions = view.getExceptions();exceptions.hasNext();){
    logException(exceptions.next());
}

Comment 11 Andrei Badea 2008-06-26 15:19:01 UTC
That is longer and more clumsy than simply

for (Throwable t : view.getExceptions()) {
    logException(t);
}

Also, how do you do

myColl.addAll(view.getExceptions());

when getExceptions() returns an iterator?

> I am not sure what makes you think I did not think of more use cases here.

The "you can always look at the DataView" kind of arguments. Again: what if I *don't* have the DataView instance
available anymore?
Comment 12 _ ahimanikya 2008-06-26 16:05:40 UTC
Hmmm...Clumsy !!

So you are saying you can't debug if you have an Iterators? And why you want to create yet another collection? Even if
you do, why can't you can iterate the iterator add all of them yourself to the new collection (cost point of view they
are will be the same). Its not only about what a developer can do and how flexible the API is to write neat code, you
also have to think about data integrity and abstraction, which is important and I felt Iterator is suffice here. 

All your arguments so far has not convinced me to return a Collection. I feel this the perfect situation where an API
developer should go with an Iterator than a Collection.

Hey, these are all your stuff, if you just want it the way you want it, I will just do it that way. But I beg to differ
here.
Comment 13 Andrei Badea 2008-06-26 16:28:01 UTC
How is data integrity affected by returning a Collection instead of an Iterator? 

Which Collection methods break abstraction?
Comment 14 _ ahimanikya 2008-06-26 18:20:11 UTC
What you think Iterators are for?  And what you understand by data abstraction ? Do you know when two components
handshake there has to be an agreement and the agreement depends on the level of comfort zone from both direction? So,
do you know there are multi level of abstraction which can be expressed in a variety of different way and variety of
different style? 

So whatever you think is correct may not be always correct, and you may not have the same prospective as the other person.

You may be correct in your prospective, but I did not see any valid argument why I should not use Iterator here. 

Comment 15 David Vancouvering 2008-06-26 18:53:50 UTC
Ahi, I have to say I don't like your proposed solution to how we can pass a collection around without having to hold on
to the "owner" of the collection.  It sounds like you are suggesting the user has to move through the iterator and build
their own copy of the collection:

ArrayList<Exception> exceptions  = new ArrayList<Exception>()
for( ; Exception e = it.next() ; it.hasNext()) {
  exceptions.add(e);
}

Yes, you could to this.  But my gut response as a user of an API is "what a pain" and "why do I have to make a copy of
this thing?".  It's expensive in terms of code verbosity, CPU time and data (making a copy).

Looking through this discussion, your main argument appears to be "Iterators are designed to be used for APIs to provide
abstraction to the underlaying collections."  Isn't the Collection interface also an abstraction?  I don't understand
the distinction that is important to you.  Why don't you feel "safe" with a Collection and you feel safe with an
Iterator, particularly when Iterator has the remove() method?  Are you wanting to prevent additions to the list?  If so
you can do Collections.unmodifiableCollection(myCollection).

As a user, I find the Iterator clumsy because I can only go through it once - it's not reusable.  And I don't want to
have to make copies.

I have been able to make it work for my particular use case, but it seems fragile - depending on changing needs, I'm
going to have to write my own code to iterate through the list and build my own collection.  It's just annoying.

You also say "hey, these are all your stuff, if you just want it the way you want it, I will just do it that way."  I
don't know about you, but I don't want to work that way.  Standard open source dialog is based on consensus, and the
normal approach I've seen is that ultimately the owner of the code makes the call, and the rest of the community has the
right to veto but only if they feel it's really serious.

In this case I would not veto the current API, I can work with it as it is, but I do want to voice my opinion that I
don't think it works best for me as the user.

So unless you've changed your mind, let's just stick with the Iterator.
Comment 16 _ ahimanikya 2008-06-26 19:17:23 UTC
To start with, I still don't understand why you have to make a copy. Anyway, I will change it to unmodifiable collection
if that works better for you. 
Comment 17 _ ahimanikya 2008-06-26 20:52:31 UTC
Changed to Collection<Throwable> getExceptions(), also adjusted db.core to reflect this

http://hg.netbeans.org/dbdataview/rev/f0fbd5495cce
Comment 18 _ ahimanikya 2008-06-27 04:49:58 UTC
This is getting interesting!!

I have given a fresh thought to this and I still strongly feel Iterator is a better candidate, though I will still
settle with Collection in this particular case, to make my user happy :)

But let me still present my analysis here.

1) Giving read only access is a different topic, which can be done in either Iterator or Collection.

2) To be able to add one or all to an emerging collection, does not really depend on who does it, whether you are
calling addAll or looping through an Iterator to add all of them yourself. If it looks Clumsy you can put the code aside
in another local method and call it as addAllSomething. Unless the Iterator of the underlaying collection is written
poorly, the CPU cycle will not be different. So there is no cost difference.

3) By returning a Collection I will be leaking the implementation specifics e.g. the caller may use instanceof and then
follow his own logic based on the underlaying Collection object, this is where the abstraction is lost. Do I think
Iterator is the best thing to use here, may be not, but this is the best alternate available to map my design thinking. 

4) To me the code level clumsiness that was raised is just a furniture issue and should not motivate an API developer to
tweak his API just because of this as a factor.

5) Last, if I have to agree with you, then I have to agree Iterators are unless and we should deprecate them in future
Java releases. Before JDK 1.5 "for(i...)" format was clumsy and now the "for(Iterator it...)" format is clumsy. So its a
matter of coding style than anything else. And its not about the coding comfort, its about my design thoughts behind it.
So, if you still have the same reaction "what a pain", then may be I did not do a good job in explaining my thoughts :)

Also I found the following articles very appropriate for the context here. 

http://www.adtmag.com/java/article.aspx?id=4859
http://www.artima.com/forums/flat.jsp?forum=106&thread=119193

http://www.artima.com/forums/flat.jsp?forum=106&thread=118774
Comment 19 David Vancouvering 2008-06-27 06:21:47 UTC
Hi, Ahi, like I said, you own this module, and it is your prerogative to design it how it seems best for you.  It is our
duty as users to provide feedback, but please consider it feedback and not a demand.

In ARC within Sun, reviewers can give two types of feedback: TCR (technical change required) and TCA (technical change
advised).  Normally there are only a few TCRs; they should be used sparingly and wisely.  Requesting a Collection vs. an
Iterator is a TCA in my mind.  

So that means you are free to say "thank you but no thanks" and that is your prerogative.  It is only rarely that I as a
member of the team and owner of the db module would feel I need to pull out the TCR stamp on any issue I have.  For
instance, internationalization is a TCR.  Threading issues (deadlocks or corruption), memory leaks and broken
functionality are TCRs.  Not Iterator vs. Collection.

That said, let me do a little rebuttal:

"2) To be able to add one or all to an emerging collection, does not really depend on who does it, whether you are
calling addAll or looping through an Iterator to add all of them yourself. If it looks Clumsy you can put the code aside
in another local method and call it as addAllSomething. Unless the Iterator of the underlaying collection is written
poorly, the CPU cycle will not be different. So there is no cost difference."

I don't understand this.  You have already built up the list of Exceptions, have you not, prior to passing me an
Iterator?  If not, where is the pending not-built list of exceptions being kept?


"3) By returning a Collection I will be leaking the implementation specifics e.g. the caller may use instanceof and then
follow his own logic based on the underlaying Collection object, this is where the abstraction is lost. Do I think
Iterator is the best thing to use here, may be not, but this is the best alternate available to map my design thinking. "

This I think is actually a potentially good point. But I'd have to be pretty nasty to do this kind of hack.  I doubt
most users of the API would go to these lengths.

Also, if you're really concerned about this, you can make it a non-modifiable collection and then who cares whether the
user knows what it's an instance of, they can't do anything with it anyway.  It's like looking into a glass house.

"4) To me the code level clumsiness that was raised is just a furniture issue and should not motivate an API developer
to tweak his API just because of this as a factor."

If we were to take this argument to its logical conclusion, then we might as well all be writing in assembly.  IMHO
expressiveness and simplicity and minimization of code (e.g. non-clumsiness) is essential for maintainability and
productivity.

"5) Last, if I have to agree with you, then I have to agree Iterators are unless and we should deprecate them in future
Java releases. Before JDK 1.5 "for(i...)" format was clumsy and now the "for(Iterator it...)" format is clumsy. So its a
matter of coding style than anything else. And its not about the coding comfort, its about my design thoughts behind it.
So, if you still have the same reaction "what a pain", then may be I did not do a good job in explaining my thoughts :)"

I actually have seen less and less use of Iterator since Collection came around, and for good reason IMHO.  It provides
a mechanism for defining operations on a list/collection that is richer and more useful than Iterator.

From your second link:

"Actually, Collection is the logical evolution of Iterator.

Because using Iterator affects its state, we want a separate instance for each use. Iterable lets us do this."

Bingo.  The problem with Iterator is it's a one-shot deal.  To get multiple shots, you have to copy it into a Collection.

The writer goes on:

"The problem with using Iterable is that some common operations require checking all elements for an external class, but
are orders of magnitude faster if performed by the Iterable instance itself: size checking is obvious, ArrayList can
return an array of its elements with just an array copy, and HashSet and sorted collections can do fast inclusion
checking. In order to have the implementation handle these cases where applicable, we need a subtype of Iterable, and we
get Collection! All of Collection's non-optional methods could be done by a utility class working with Iterable, but
would often be slower.

Thus, Collection fills the same role as Iterable, and the only reason Iterable is now seperate is the new for loop."

If I can translate, this means Collection is more flexible.  I can do size() and get an answer reasonably fast.  I can
do contains() and get a pretty fast answer.  None of these functions are available on Iterator or Iterable.  Collection
provides a richer interface.

Comment 20 _ ahimanikya 2008-06-27 07:30:08 UTC
Sure, I agree with you on who makes the call. Good to know about TCR and TCA concept.

[Ahi]"2) To be able to add one or all to an emerging collection, does not really depend on who does it, whether you are
calling addAll or looping through an Iterator to add all of them yourself. If it looks Clumsy you can put the code aside
in another local method and call it as addAllSomething. Unless the Iterator of the underlaying collection is written
poorly, the CPU cycle will not be different. So there is no cost difference."

[David]I don't understand this.  You have already built up the list of Exceptions, have you not, prior to passing me an
Iterator?  If not, where is the pending not-built list of exceptions being kept?

[Ahi] Correct, the point was, if you wanted to use addAll to take the returns Exceptions to another collection then
someone has to loop. No? Whether it is provided the JDK or you do it locally. So, where is the extra CPU time here?
 
[Ahi]"3) By returning a Collection I will be leaking the implementation specifics e.g. the caller may use instanceof and
then follow his own logic based on the underlaying Collection object, this is where the abstraction is lost. Do I think
Iterator is the best thing to use here, may be not, but this is the best alternate available to map my design thinking. "

[David]This I think is actually a potentially good point. But I'd have to be pretty nasty to do this kind of hack.  I
doubt most users of the API would go to these lengths.

Also, if you're really concerned about this, you can make it a non-modifiable collection and then who cares whether the
user knows what it's an instance of, they can't do anything with it anyway.  It's like looking into a glass house.

[Ahi] Well you missed my point slightly. The abstraction that Iterator provides, Collection definitely does not do that,
regardless of whether you make it read only or not. But anyway, in this specific situation it does not really matter
much. So I have decided to go with your TCA. 

[Ahi]"4) To me the code level clumsiness that was raised is just a furniture issue and should not motivate an API
developer to tweak his API just because of this as a factor."

[David]If we were to take this argument to its logical conclusion, then we might as well all be writing in assembly. 
IMHO expressiveness and simplicity and minimization of code (e.g. non-clumsiness) is essential for maintainability and
productivity.

[Ahi] Huh ?!? So, is using Iterators is that clumsy?

[David]I actually have seen less and less use of Iterator since Collection came around, and for good reason IMHO.  It
provides a mechanism for defining operations on a list/collection that is richer and more useful than Iterator.

[Ahi] Yes, may be people are using less Iterator for convenience, read article 1, which is very similar to my thought
process. The quote that picked up was a reader feedback in article2, but I agree more with what the original post. 

[David]Thus, Collection fills the same role as Iterable, and the only reason Iterable is now separate is the new for loop."

If I can translate, this means Collection is more flexible.  I can do size() and get an answer reasonably fast.  I can
do contains() and get a pretty fast answer.  None of these functions are available on Iterator or Iterable.  Collection
provides a richer interface.

[Ahi] Again you are talking about convenience, Collection is good within your component boundary, unless the API handing
over a collection and no longer holds a reference to it. Iterator is good when you are handing over control to another
component. By giving away a reference to the underlaying Collection is about giving more control to the caller,
specifically when you are also holding a reference, I would like see the API developer is more conservative here. 
Comment 21 David Vancouvering 2008-06-27 08:00:42 UTC
It seems your main point is that handing an Iterator to another component rather than a Collection is good because you
are not handing a direct reference to the collection.  The reasons this could be a bad thing are, as far as I can see:

- The caller now holds a reference, so you have less control over when you can modify the collection without causing
issues for the caller.  In other words, it's more like by-value than by-reference.  But in most cases it isn't an actual
copy - most iterators hold a reference to the underlying collection and are moving through the existing collection, no?
 So if you hand an iterator to another component, what happens if they next() through half of it, and then you change
your collection?

From discussion in your last link: "My gut feel is that although true in theory, in practise this does not make much
difference. The vast majority of Iterator instances are themselves created from the iterator() method of Collection, so
whenever you have an Iterator you usually also have the Collection it came from. (Perhaps you could use your Python
script to test this assertion?). Yes, you can create independent implementations of Iterator that have nothing to do
with a Collection, but this is not frequently done in practice."


- The caller now holds a reference, so you can't control when your collection gets GCd, and you could have memory leaks.
 But the same issue holds if you pass an iterator, because the iterator implementation likely holds a reference to the
underlying collection, unless you do a full copy.

- The caller now holds a reference, and can make changes.  But we've already discussed this one to death.

So, maybe I'm missing something: in what way does an iterator give you a better boundary between your component and
another?  The only way this would really be true is if you have the iterator refer to a copy of your collection rather
than the original collection.  And if that's the case, why not just make a copy of the collection in the first place,
and return that, instead of an Iterator, which is, as you put it, less convenient.

I think convenience is valuable, if there is no clear and present danger to providing it.  As far as I can tell there is
no clear and present danger to providing a Collection instead of an Iterator - in either case the safest thing to do is
make a copy before returning it to the caller, although I suspect just making it read-only is sufficient.

Comment 22 _ ahimanikya 2008-06-27 08:32:48 UTC
[David]So if you hand an iterator to another component, what happens if they next() through half of it, and then you
change your collection?

[Ahi]This should not be an issue. I thought, the contract is clearly defined in the Iterator API, no?

I don't think I am clearly getting your point you are making in your previous post. The author was criticizing the Java
Collection implementations and complaining about the fact the iterators are not being external one, but that a different
topic.

I was not referring to object references, I was speaking more about functional design prospective. What if the user did
a instance of and found a ArrayList and then cast and directly manipulate my Collection, thats convenient (may be), but
thats not what I want user to do, so I may go with an Iterator here.

P.S: The breadth of the discussion has gone beyond getExceptions(), so when I am making my point, I am keeping more
generic situations in my mind.