Bug 45359

Summary: Rename poorly-named POD* classes
Product: WebKit Reporter: Kenneth Russell <kbr>
Component: WebCore Misc.Assignee: Kenneth Russell <kbr>
Status: ASSIGNED ---    
Severity: Normal CC: abarth, cmarrin, darin, fishd, jamesr, senorblanco, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 44729    

Description Kenneth Russell 2010-09-07 19:35:25 PDT
The PODArena, PODRedBlackTree and PODIntervalTree classes recently added for https://bugs.webkit.org/show_bug.cgi?id=44729 are poorly named; they do not strictly allocate and operate upon only Plain Old Data. It has been pointed out that the naming convention does not add any value; the classes should be renamed.
Comment 1 Chris Marrin 2010-09-08 07:37:07 PDT
I want to put in my vote for either getting rid of the prefix entirely. If it's unsavory to have a simple Arena class (because of the Arena.h file elsewhere) then perhaps it can be called RBTreeArena or something like that.

Just some thoughts.
Comment 2 Darin Fisher (:fishd, Google) 2010-09-08 09:41:43 PDT
Another suggestion:

  PODArena -> ObjectArena (since it only knows how to allocate objects of a fixed type)
  PODRedBlackTree -> RedBlackTree
  PODIntervalTree -> IntervalTree

One question is what to do with PODInterval.  We could rename that Interval, but then it seems like we might be taking over a name that is quite generic sounding for this specific use.  Interval cannot be an inner class of IntervalTree since it is a template argument to IntervalTree.  Thoughts?
Comment 3 Chris Marrin 2010-09-08 10:30:57 PDT
(In reply to comment #2)
> Another suggestion:
> 
>   PODArena -> ObjectArena (since it only knows how to allocate objects of a fixed type)
>   PODRedBlackTree -> RedBlackTree
>   PODIntervalTree -> IntervalTree
> 
> One question is what to do with PODInterval.  We could rename that Interval, but then it seems like we might be taking over a name that is quite generic sounding for this specific use.  Interval cannot be an inner class of IntervalTree since it is a template argument to IntervalTree.  Thoughts?

Currently these classes have a specific use. They are used for an accelerated 2D tesselator, right? Perhaps we could identify them as such, e.g., TessArena, TessRBTree, TessIntervalTree. Or is that too specific for their ultimate purpose? BTW, I think renaming RedBlackTree to RBTree is reasonable. It seems to be a common shorthand.
Comment 4 Darin Fisher (:fishd, Google) 2010-09-08 11:17:55 PDT
That suggestion sounds pretty good to me, but I defer to Ken in case I'm overlooking some intended use cases.  Giving them a "namespace" like that means that we would also not need to prematurely add code to call destructors since we would very clearly be making these data structures domain specific (minimizing the risk of misuse).