Claudeの学習モードを使ってコードのリファクタリングをした
前回のブログ記事でちょっとだけ触れたClaude Codeの学習モードについて、ちょうどいい感じのコードが上がってきたのでやってみました。
今回はClaude Codeではなく、Claudeのチャットでやってます。
やっぱりなんか………チャットのClaudeの方が賢い気がしているのですがそういうモードだからでしょうか?
リファクタ前のコードはこれ(by Claude Code)
const moveImage = ( index: number, action: ImageOrderAction ) => {
if ( action === 'moveUp' && index > 0 ) {
const newImages = [ ...images ];
[ newImages[ index ], newImages[ index - 1 ] ] = [
newImages[ index - 1 ],
newImages[ index ],
];
setAttributes( { images: newImages } );
} else if ( action === 'moveDown' && index < images.length - 1 ) {
const newImages = [ ...images ];
[ newImages[ index ], newImages[ index + 1 ] ] = [
newImages[ index + 1 ],
newImages[ index ],
];
setAttributes( { images: newImages } );
}
};
-
moveUp
とmoveDown
で移動先違うだけじゃん? - そもそも見づらい。何してるのかは読んだらわかるけど、読むコストが高い
結構はずかしいやりとりしてますがやりとりの様子はこちらで見れます。
いつまで残るかはわからないので、ちょっとずつピックアップ
最初にどういうことやりたいかを言います。
細かくやりとりに付き合ってくれます。言葉の意味も結構厳密に言ってくれるのは良いですね。
合ってること言ってるのに「惜しい!」って言われてウンウン悩んで合ってるんじゃないん??って聞いたら「そうです!正解です!」って言われてイラッとしたところ。
ヒントでsplice
使う的なこと言われたけど、リファレンス見ながら「ん? toSpliced
の方がいいのでは………?」って悩み始めて言ってみたところ。今考えても普通に (CTOから補足でsplice
でやった方がいいやつtoSpliced
の方がベターって言われたので合ってた!!!)ですが、Claudeくんは「やってみな!!」ってします。これはこれでメソッドの勉強になっていいと思います。
だいぶ混乱してる図。
眠くてギブしかけたところ。CTOにここ見て爆笑されました。
ギブする前に終わったのは以下のコードです。
const moveImageItem = (images, fromIndex, toIndex) => {
if (fromIndex < 0 || fromIndex >= images.length ||
toIndex < 0 || toIndex >= images.length ||
fromIndex === toIndex ||
!Number.isInteger(toIndex) || !Number.isInteger(fromIndex)) {
return images;
}
const itemToMove = images[fromIndex];
const removed = images.toSpliced(fromIndex, 1);
const adjustedToIndex = fromIndex < toIndex ? toIndex - 1 : toIndex;
const added = removed.toSpliced(adjustedToIndex, 0, itemToMove);
return added;
};
const moveImage = (index: number, action: ImageOrderAction) => {
let moveTo;
if (action === 'moveUp') {
moveTo = index - 1;
} else if (action === 'moveDown') {
moveTo = index + 1;
}
setAttributes({ images: moveImageItem(images, index, moveTo) });
};
翌日、ただ単に前後の入れ替えだけだったらわざわざtoSpliced
しなくていいんじゃないの???って思って、再度リファクタリング開始!
Array.prototype.with()
は初見だったので調べたんですが、かなり良さそうと思い採用!
Claudeくん、時々合ってるか間違ってるか言わずに質問投げてくるのでちょっとドキドキします。
え、あ、え………ま、間違ってる………?って思いながら説明したら、素晴らしい理解ですね!実際に頭の中で実行してくれて、結果も正確に把握できています。
って褒めてくるのでほっとする。
で、なんやかんや境界チェックだなんだを挟み、やり切ったコードはこちら。
const moveImageItem = (images, fromIndex, toIndex) => {
// toIndexとfromIndexの要素を入れ替える。無効なインデックスの場合は元の配列を返す
try {
return images.with(toIndex, images[fromIndex]).with(fromIndex, images[toIndex]);
} catch {
return images;
}
};
const moveImage = (index: number, action: ImageOrderAction) => {
let moveTo;
if (action === 'moveUp') {
moveTo = index - 1;
} else if (action === 'moveDown') {
moveTo = index + 1;
}
setAttributes({ images: moveImageItem(images, index, moveTo) });
};
ちなみにここからやっぱ try ... catch
ヤダなと思って今度はClaude Codeとやりとりして、早期リターンに変えてます。
const moveImageItem = (
images: Image[],
fromIndex: number,
toIndex: number
) => {
// インデックスが負の値の場合は何もしない
if ( fromIndex < 0 || toIndex < 0 ) {
return images;
}
// 配列の範囲外の場合は何もしない
if ( fromIndex >= images.length || toIndex >= images.length ) {
return images;
}
return images
.with( toIndex, images[ fromIndex ] )
.with( fromIndex, images[ toIndex ] );
};
また、moveUp
とmoveDown
で moveTo
出し分けもしてましたが、moveTo
のチェックが入って逆に冗長になったのでmoveImage
もこのように書き換えました。
const moveImage = ( index: number, action: ImageOrderAction ) => {
if ( action === 'moveUp' ) {
setAttributes( {
images: moveImageItem( images, index, index - 1 ),
} );
} else if ( action === 'moveDown' ) {
setAttributes( {
images: moveImageItem( images, index, index + 1 ),
} );
}
};
これなら見通しもよくて良いですね。
私は結構口語もチャットにわって書いちゃうタイプなのですが、どういうところまではわかってるけどここが間違ってるなってClaude側も察する(って言い方が合ってるのか謎だけど)ようで、わりとわからないポイントをしっかりサポートしてくれる気がします。
自分で勉強進めるための強い味方になってくれそうです。
Discussion