Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 326607 - Use descriptive type rather than Map<String, Object> to represent an EDC frame
Summary: Use descriptive type rather than Map<String, Object> to represent an EDC frame
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-debug-edc (show other bugs)
Version: 8.0   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Ken Ryall CLA
QA Contact: Ken Ryall CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-29 18:40 EDT by John Cortell CLA
Modified: 2010-09-29 20:23 EDT (History)
0 users

See Also:
john.cortell: review? (ken.ryall)


Attachments
Solution (5.75 KB, patch)
2010-09-29 18:41 EDT, John Cortell CLA
john.cortell: iplog-
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Cortell CLA 2010-09-29 18:40:24 EDT
In the EDC API, a frame object is described using a property bag (a Map<String,Object>). E.g.,

 // EDC adopter must implement this
  protected abstract List<Map<String, Object>> computeStackFrames();

If this method would at least describe in javadoc what sort of properties are expected in the bag, then the situation wouldn't be so bad; the EDC adopter would at least know what to provide. But it doesn't. And of course, the property bag is passed around quite a bit. So, to duplicate that documentation in all those places is clearly not a good approach. For this reason, I dislike this sort of API. A slight variation can keep the flexibility intended by the API but make it much more intuitive to use:

  protected abstract List<EdcStackFrame> computeStackFrames();

   /**
    * A stack frame described as one or more of the following 
    * properties, plus any additional custom properties.
    * 
    * <ul>
    * <li>{@link StackFrameDMC#LEVEL_INDEX}
    * <li>{@link StackFrameDMC#ROOT_FRAME}
    * ...
    * </ul>
    */
   public class EdcStackFrame  {
      public Map<String, Object> props;
   }

Clearly, this would be API breaking stuff, but I think we're still at a point where adoption is low and this sort of improvement is not only doable but necessary in order to make the framework more usable. 

This improved pattern may be applied to other aspects of EDC. I see other things being passed around as opaque property bags (threads, breakpoints). Any such adjustments will be done via dedicated bugs reports. This one only addresses EDC stack frames.
Comment 1 John Cortell CLA 2010-09-29 18:41:04 EDT
Created attachment 179909 [details]
Solution
Comment 2 John Cortell CLA 2010-09-29 18:44:50 EDT
Committed to HEAD. Ken, please review.
Comment 4 CDT Genie CLA 2010-09-29 20:23:02 EDT
*** cdt cvs genie on behalf of jcortell ***
Bug 326607:  Use descriptive type rather than Map&lt;String, Object&gt; to represent an EDC frame

[*] Stack.java 1.37 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/edc/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/services/Stack.java?root=Tools_Project&r1=1.36&r2=1.37