[webkit-reviews] review granted: [Bug 28355] Replace MAX()/MIN() macros with type-safe std::max()/min() templates : [Attachment 34925] Patch v1

[bugzilla-daemon at webkit.org]

mitz at webkit.org has granted David Kilzer (ddkilzer) <ddkilzer at webkit.org>'s
request for review:
Bug 28355: Replace MAX()/MIN() macros with type-safe std::max()/min() templates
https://bugs.webkit.org/show_bug.cgi?id=28355

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

------- Additional Comments from mitz at webkit.org
> +	   <http://webkit.org/b/00000> Replace MAX()/MIN() macros with
type-safe std::max()/min() templates

Presumably this will transform into this bug’s URL?

> +	   * platform/graphics/mac/SimpleFontDataMac.mm: Added using
> +	   std::max statment.

Typo: statment.

In new files, it is common to just add “using namespace std;”.

> +	   <http://webkit.org/b/00000> Replace MAX()/MIN() macros with
type-safe std::max()/min() templates

URL…

> +	       deliveryBytes =
static_cast<int32>(min(static_cast<NSUInteger>(deliveryBytes), [subdata
length]));

This is an odd combination of casts. You can do “min<int32>(deliveryBytes,
[subdata length])” instead.

> -		       dataLength = MIN((unsigned)[contentLength intValue],
dataLength);
> +		       dataLength = min(static_cast<unsigned>([contentLength
intValue]), dataLength);

I prefer min<unsigned>(…).

> -	       imageSize.width = MAX(MAX_DRAG_LABEL_WIDTH + DRAG_LABEL_BORDER_X
* 2.0f, MIN_DRAG_LABEL_WIDTH_BEFORE_CLIP);
> +	       imageSize.width = max(MAX_DRAG_LABEL_WIDTH + DRAG_LABEL_BORDER_X
* 2.0f, MIN_DRAG_LABEL_WIDTH_BEFORE_CLIP);

While you’re here, you may just drop the “.0f”.

> -	       imageSize.width = MAX(labelSize.width + DRAG_LABEL_BORDER_X *
2.0f, urlStringSize.width + DRAG_LABEL_BORDER_X * 2.0f);
> +	       imageSize.width = max(labelSize.width + DRAG_LABEL_BORDER_X *
2.0f, urlStringSize.width + DRAG_LABEL_BORDER_X * 2.0f);

Ditto.

> +	   maxWidth = min(static_cast<CGFloat>(400), windowFrame.size.width);

Can be done as min<CGFloat>(…).

r=me but please consider at least changing to “using namespace std;”.