Skip to content

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Dec 2, 2015

The Is* type checking functions in node_util.cc are mostly the same boilerplate. This commit defines them using a macro.

Refs: #4100
R=@bnoordhuis

@Fishrock123 Fishrock123 added the c++ Issues and PRs that require attention from people who are familiar with C++. label Dec 2, 2015
@mscdex mscdex added the util Issues and PRs related to the built-in util module. label Dec 2, 2015
X(IsTypedArray)
X(IsArrayBuffer)
X(IsDataView)
#undef X
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can improve on this with a map macro:

#define VALUE_METHOD_MAP(V) \
  V(isArrayBuffer, IsArrayBuffer) \
  V(isDataView, IsDataView) \
  // etc.

Then:

#define V(_, ucname) \
  static void ucname(const FunctionCallbackInfo<Value>& args) { \
  // etc.
VALUE_METHOD_MAP(V)
#undef V

And further down below:

#define V(lcname, ucname) env->SetMethod(target, #lcname, ucname);
  VALUE_METHOD_MAP(V)
#undef V

@cjihrig
Copy link
Contributor Author

cjihrig commented Dec 2, 2015

@bnoordhuis updated

@jasnell
Copy link
Member

jasnell commented Dec 3, 2015

LGTM

@jasnell
Copy link
Member

jasnell commented Dec 3, 2015

Marking this as an LTS watch but I'm not convinced yet we should land it there.

@bnoordhuis
Copy link
Member

LGTM

@cjihrig
Copy link
Contributor Author

cjihrig commented Dec 4, 2015

@cjihrig cjihrig closed this Dec 4, 2015
cjihrig added a commit to cjihrig/node that referenced this pull request Dec 4, 2015
The Is* type checking functions in node_util.cc are mostly
the same boilerplate. This commit defines them using a macro.

Refs: nodejs#4100
PR-URL: nodejs#4118
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@cjihrig cjihrig merged commit 46af397 into nodejs:master Dec 4, 2015
@cjihrig cjihrig deleted the macros branch December 4, 2015 03:35
cjihrig added a commit that referenced this pull request Dec 5, 2015
The Is* type checking functions in node_util.cc are mostly
the same boilerplate. This commit defines them using a macro.

Refs: #4100
PR-URL: #4118
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@rvagg rvagg mentioned this pull request Dec 17, 2015
@MylesBorins
Copy link
Contributor

this relies on #4100 to land

cjihrig added a commit to cjihrig/node that referenced this pull request Jan 7, 2016
The Is* type checking functions in node_util.cc are mostly
the same boilerplate. This commit defines them using a macro.

Refs: nodejs#4100
PR-URL: nodejs#4118
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Conflicts:
	src/node_util.cc
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
The Is* type checking functions in node_util.cc are mostly
the same boilerplate. This commit defines them using a macro.

Refs: #4100
PR-URL: #4118
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Conflicts:
	src/node_util.cc
@MylesBorins MylesBorins mentioned this pull request Jan 19, 2016
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
The Is* type checking functions in node_util.cc are mostly
the same boilerplate. This commit defines them using a macro.

Refs: nodejs#4100
PR-URL: nodejs#4118
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants