[webkit-changes] [58748] trunk/WebCore

[Nikolas Zimmermann]

Good evening Alexander,

just had a quick glance over your last refactoring patch.
I don't see the point in creating a new struct and just delegating the  
calls into the InspectorCSSStore, to access public variables.
The code in InspectorDOMAgent is harder to read now. Why don't you  
encapsulate this properly?

> +struct InspectorCSSStore {
> +    InspectorCSSStore();
> +    ~InspectorCSSStore();
> +    void reset();
> +
> +    StyleToIdMap styleToId;
> +    IdToStyleMap idToStyle;
> +    RuleToIdMap ruleToId;
> +    IdToRuleMap idToRule;
> +    IdToDisabledStyleMap idToDisabledStyle;
> +    RefPtr<CSSStyleSheet> inspectorStyleSheet;
> +};
> +
>
>  void InspectorDOMAgent::applyStyleText(long callId, long styleId,  
> const String& styleText, const String& propertyName)
>  {
> -    IdToStyleMap::iterator it = m_idToStyle.find(styleId);
> -    if (it == m_idToStyle.end()) {
> +    IdToStyleMap::iterator it = cssStore()->idToStyle.find(styleId);
> +    if (it == cssStore()->idToStyle.end()) {
>          m_frontend->didApplyStyleText(callId, false,  
> ScriptValue::undefined(), m_frontend->newScriptArray());
>          return;
> +    IdToDisabledStyleMap::iterator disabledIt = cssStore()- 
> >idToDisabledStyle.find(styleId);
> +    if (disabledIt == cssStore()->idToDisabledStyle.end())
> +        disabledIt = cssStore()->idToDisabledStyle.set(styleId,  
> DisabledStyleDeclaration()).first;
>
Just selected some examples, code like the one above could go into the  
InspectorCSSStore directly.
Note that I'm not familiar at all with the Inspector code, but it  
looks like this more convoluted then it needs to be :-)

Cheers,
Niko


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-changes/attachments/20100504/d43805de/attachment.html>