[
https://issues.apache.org/jira/browse/WAVE-263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13029074#comment-13029074
]
David Hearnden commented on WAVE-263:
-------------------------------------
I don't think it's too difficult to do it all in one go, but 3) sounds like the
right choice if this is to be done in two steps.
Most call sites use getAllReplyThreads(), already, which helps. There are only
two things that use the other methods: the robot API, and the client's
deprecated ConversationRenderer. The latter is distinguishes inline and
non-inline in order to put private conversations between them. That can safely
be changed to having the private conversations follow all replies, thus
removing the distinction.
That then leaves only the robot API, it looks like there's only three places,
two of which just iterates over inline reply threads followed by iterating over
reply threads (so should be replaced with all reply threads), leaving only one
real use case over the whole codebase for getInlineReplyThreads() (in
EventDataConverterV22*). The only thing that is different from
getAllReplyThreads is that that method needs/wants to know the inline location
of each thread.
So I suggest the following plan moving forward.
*) Make the above changes, removing all calls to getReplyThreads, and leaving
only one call to getInlineReplyThreads; then
*) Rename getInlineReplyThreads to locateReplyThreads, to indicate that it is
not a cheap method, and have it return a map of thread -> location rather than
a list, which is more appropriate for the one use case that uses that method.
*Note that that robot method has quadratic complexity (O(replies^2)) because it
calls getInlineReplyThreads once for each thread, and getInlineReplyThreads is
O(docsize) (which is O(replies)) because it dynamically crawls the blip for
anchor locations every time it is called.
> ConversationThread.isInline is still used in some parts of the codebase
> -----------------------------------------------------------------------
>
> Key: WAVE-263
> URL: https://issues.apache.org/jira/browse/WAVE-263
> Project: Wave
> Issue Type: Bug
> Reporter: David Hearnden
> Assignee: Michael MacFadden
> Priority: Minor
>
> In Google Wave, there were four types of reply threads:
> inline=false, no content anchor => an indented reply thread outside the
> parent blip.
> inline=false, with content anchor => corrupt data, but treated anyway as
> above
> inline=true, no content anchor => unanchored inline reply, rendered at the
> end of the parent blip (inside).
> inline=true, with content anchor => anchored inline reply, rendered at the
> content anchor (inside).
> In Undercurrent / WIAB, this distinction based on the inline attribute in the
> manifest is no longer made. Reply threads are just reply threads. Any reply
> thread can have an anchor in its parent blip's content, and if there is such
> an anchor, the thread is rendered there, otherwise it is rendered at the end
> of the parent blip's content. The rendering and behaviour of the two cases
> is identical (unlike Google Wave). The only difference is the location where
> the thread is rendered. Essentially, this means that "indented"
> replies, as opposed to "inline" replies, are gone.
> All code that is causing divergent behaviour based on
> ConversationThread.isInline() needs to be fixed. In most cases, the
> expression:
> ConversationThread.isInline()
> can be replaced with:
> true
> followed by consequent refactoring and dead-code removal etc.
> ---
> Issue imported from
> http://code.google.com/p/wave-protocol/issues/detail?id=264
> Owner: [email protected]
> Cc: [email protected]
> Cc: vega113
> Label: Type-Defect
> Label: Priority-Medium
> Stars: 1
> State: open
> Status: New
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
