Skip to content

Conversation

@kio-watanabe
Copy link
Contributor

FUNCTION SUBSTITUTE and FUNCTION SUBSTITUTE-CASE are added.
Four failing tests now pass.

Copy link
Contributor

@yutaro-sakamoto yutaro-sakamoto left a comment

Choose a reason for hiding this comment

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

一旦レビューは以上とします。修正を完了したらコメントをください。アルゴリズムの正確性を検証します。

}
}

byte[] p1 = fields[0].getDataStorage().getByteArray();
Copy link
Contributor

@yutaro-sakamoto yutaro-sakamoto Nov 2, 2023

Choose a reason for hiding this comment

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

CobolDataStorageクラスのgetByteArray()メソッドは使用しないでください。このPRでついでに、このメソッドをCobolDataStorageから消してもらえますか。
集団項目や部分参照のことを考慮すると、このメソッドの使用は(よほど注意深くコーディングしない限り)不具合を引き起こします。
ただし、CobolDataStorageクラスのgetByteArray(int, int)は、(性能面を十分に検討した上で)使用して良いです。

Copy link
Contributor

Choose a reason for hiding this comment

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

@kio-watanabe
すみません、この指摘は誤りでした。修正は不要です。

Copy link
Contributor

Choose a reason for hiding this comment

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

@kio-watanabe
すみません、やはりgetByteArray()は使用しないように修正してください

int varsize = p1.length;
int pi1 = 0;
int calcsize = 0;
int found = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

このfoundの型はbooleanに変更することを推奨します。

Comment on lines 2234 to 2235
AbstractCobolField field = CobolFieldFactory.makeCobolField(0, (CobolDataStorage) null, attr);
field.setSize(calcsize);
Copy link
Contributor

Choose a reason for hiding this comment

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

単に

AbstractCobolField field = CobolFieldFactory.makeCobolField(calcsize, (CobolDataStorage) null, attr);

と変更するのが良いでしょう

@yutaro-sakamoto yutaro-sakamoto merged commit c1a83e5 into opensourcecobol:develop Nov 16, 2023
@yutaro-sakamoto yutaro-sakamoto mentioned this pull request Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants