Some cleanup for xfa_expression/xfa_simpleexpression

Some cleanup for xfa_expression/xfa_simpleexpression

R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/457f421dab11104402be34a55613ffe20d44bdda

[Oliver Chang, 2016-03-03 00:09:37]

Description was changed from

==========
Some cleanup for xfa_expression.

R=tsepez@chromium.org
==========

to

==========
Some cleanup for xfa_expression.

R=tsepez@chromium.org
==========

[Oliver Chang, 2016-03-03 00:09:37]

ochang@chromium.org changed reviewers:
- tsepez@chromium.org

[Oliver Chang, 2016-03-03 00:27:47]

Description was changed from

==========
Some cleanup for xfa_expression.

R=tsepez@chromium.org
==========

to

==========
Some cleanup for xfa_expression/xfa_simpleexpression

R=tsepez@chromium.org
==========

[Oliver Chang, 2016-03-03 00:35:33]

ochang@chromium.org changed reviewers:
+ tsepez@chromium.org

[Oliver Chang, 2016-03-03 00:35:34]

Tom, please take a look. Some cleanup.

[Tom Sepez, 2016-03-03 00:48:15]

lgtm

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
File xfa/src/fxfa/src/fm2js/xfa_expression.cpp (right):

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:53: int32_t index = 0;
nit: can you make this a for-loop while you're at it( and remove the local expc
since GetSize() should be a const method)?

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:56:
reinterpret_cast<CXFA_FMExpression*>(m_pExpressions->GetAt(index));
nit: no need for |e|, just |delete reinterpret_cast<...|.

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:105: int32_t expc =
m_pExpressions->GetSize();
same here

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:232: int32_t expc =
m_pExpressionList->GetSize();
and here.

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:251: CXFA_FMExpression* e =
and here.

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:266: CXFA_FMExpression* e =
and here.

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:545: int32_t size =
m_pAccessors->GetSize();
and here.  Maybe there's some way to consolidate all this code.

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:575: int32_t index = 0;
ditto

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:629: int32_t iSize =
m_pAccessors->GetSize();
ditto. sheesh.

[Oliver Chang, 2016-03-03 01:12:00]

Patchset #3 (id:40001) has been deleted

[Oliver Chang, 2016-03-03 01:16:38]

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
File xfa/src/fxfa/src/fm2js/xfa_expression.cpp (right):

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:53: int32_t index = 0;
On 2016/03/03 00:48:15, Tom Sepez wrote:
> nit: can you make this a for-loop while you're at it( and remove the local
expc
> since GetSize() should be a const method)?

Done.

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:56:
reinterpret_cast<CXFA_FMExpression*>(m_pExpressions->GetAt(index));
On 2016/03/03 00:48:15, Tom Sepez wrote:
> nit: no need for |e|, just |delete reinterpret_cast<...|.

Done.

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:232: int32_t expc =
m_pExpressionList->GetSize();
On 2016/03/03 00:48:15, Tom Sepez wrote:
> and here.

ditto

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:251: CXFA_FMExpression* e =
On 2016/03/03 00:48:15, Tom Sepez wrote:
> and here.

ditto

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:266: CXFA_FMExpression* e =
On 2016/03/03 00:48:15, Tom Sepez wrote:
> and here.

ditto

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:545: int32_t size =
m_pAccessors->GetSize();
On 2016/03/03 00:48:15, Tom Sepez wrote:
> and here.  Maybe there's some way to consolidate all this code.

ditto

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:575: int32_t index = 0;
On 2016/03/03 00:48:15, Tom Sepez wrote:
> ditto

ditto

https://codereview.chromium.org/1764473002/diff/20001/xfa/src/fxfa/src/fm2js/...
xfa/src/fxfa/src/fm2js/xfa_expression.cpp:629: int32_t iSize =
m_pAccessors->GetSize();
On 2016/03/03 00:48:15, Tom Sepez wrote:
> ditto. sheesh.

ditto

[Tom Sepez, 2016-03-03 01:18:19]

lgtm

[Oliver Chang, 2016-03-03 17:14:04]

Description was changed from

==========
Some cleanup for xfa_expression/xfa_simpleexpression

R=tsepez@chromium.org
==========

to

==========
Some cleanup for xfa_expression/xfa_simpleexpression

R=tsepez@chromium.org

Committed:
https://pdfium.googlesource.com/pdfium/+/457f421dab11104402be34a55613ffe20d44...
==========

[Oliver Chang, 2016-03-03 17:14:06]

Committed patchset #3 (id:60001) manually as
457f421dab11104402be34a55613ffe20d44bdda (presubmit successful).