回答編集履歴
1
手直ししようと試みたが限界
answer
CHANGED
|
@@ -4,4 +4,35 @@
|
|
|
4
4
|
LineNum=buffer.size();
|
|
5
5
|
String[] stringArr = buffer.toArray(new String[0]);
|
|
6
6
|
```
|
|
7
|
-
これ以降bufferという変数が登場しません。ファイルの文字列をListと配列で二重に持っており、無駄です。
|
|
7
|
+
これ以降bufferという変数が登場しません。ファイルの文字列をListと配列で二重に持っており、無駄です。
|
|
8
|
+
そもそもこの処理は、ファイルの内容を全部文字列としてメモリに乗せることになりますから、ファイル自体が大きい場合はその対処を行ったとしてもメモリエラーになるかもしれません。Listで全部一気に取り出すのではなく、行ごとに処理したほうが賢明かと思います。
|
|
9
|
+
|
|
10
|
+
そもそもコードが非常に読みづらいです。ネストは深いし、マジックナンバーが多数登場するし、無駄な記述が多いし、変数名の意味が分かりにくいし、他多数のツッコミどころにより何をしたいのかが本当に読みづらいです。
|
|
11
|
+
とりあえずメソッド最初の変数宣言は全部消したいです。配列で定義されているものはLexerクラスのstatic定数にして、他の変数はスコープを狭くしたいので、最初から書いてみると
|
|
12
|
+
```java
|
|
13
|
+
public void run(final String inputFileName, final String outputFileName) {
|
|
14
|
+
try {
|
|
15
|
+
// ファイル読み込み部分省略
|
|
16
|
+
for(int i=0;i<lineNum;i++) { //変数は小文字始まりのものにしたほうが良い
|
|
17
|
+
arr_data[i] = new Data(); //クラス名は大文字始まりで定義したほうが良い
|
|
18
|
+
arr_data[i].lineNumber = i + 1;;
|
|
19
|
+
//44というマジックナンバーは避け、mark配列の要素を回すということを明示化
|
|
20
|
+
for(int j = 0; j < mark.length; j++) {
|
|
21
|
+
int mPoint=stringArr[i].indexOf(mark[j]);
|
|
22
|
+
if (mPoint == -1) {
|
|
23
|
+
continue; //見つからないならさっさと次に行く
|
|
24
|
+
}
|
|
25
|
+
int lQuote = stringArr[i].indexOf("'");
|
|
26
|
+
int rQuore = stringArr[i].lastIndexOf("'");
|
|
27
|
+
//同じ文字を、indexOfで見つかって、lastIndexOfで見つからない、ということは絶対にない。
|
|
28
|
+
//そのため、両方が-1であるかどうか、などの判定は不要
|
|
29
|
+
if (lQuote != -1 && (lQuote <= mPoint && mPoint <= rQuote)) {
|
|
30
|
+
continue; //クォーテーションマークで囲われてたら次に行く
|
|
31
|
+
}
|
|
32
|
+
/* ここからは何をしたいのか本当に読み取れません。正直上の変更も正しいのか微妙です。
|
|
33
|
+
ifとかforとかがどこまでの括りなのかわからないので。 */
|
|
34
|
+
}
|
|
35
|
+
}
|
|
36
|
+
}
|
|
37
|
+
}
|
|
38
|
+
```
|