[webkit-reviews] review denied: [Bug 100927] Improve ContentTypeParser, so that it could be used to validate mime type according to RFC : [Attachment 171796] Patch 1

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Nov 1 10:14:44 PDT 2012


Alexey Proskuryakov <ap at webkit.org> has denied Alexander Shalamov
<alexander.shalamov at gmail.com>'s request for review:
Bug 100927: Improve ContentTypeParser, so that it could be used to validate
mime type according to RFC
https://bugs.webkit.org/show_bug.cgi?id=100927

Attachment 171796: Patch 1
https://bugs.webkit.org/attachment.cgi?id=171796&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=171796&action=review


This interface seems overly complicated. The task is to tell if a string is
valid, so the client should just call a function. Figuring out how to combine
the templates into something usable is an unnecessary complication in this
case.

If templates are desired for implementation, that could be an implementation
detail, no need to expose that at call sites.

> Source/WebCore/platform/network/ContentTypeParser.cpp:186
> +    const unsigned contentTypeLength = contentType.length();

As a matter of coding style, we don't use const with local variables.

> Source/WebCore/platform/network/ContentTypeParser.h:82
> -    KeyValuePairs m_parameters;
> -    String m_mimeType;
> +    String m_contentType;
> +    mutable KeyValuePairs m_parameters;
> +    mutable String m_mimeType;

If "ContentTypeParser" encapsulates parsed results, then it's not a parser, but
a "ParsedContentType".


More information about the webkit-reviews mailing list