概要
汚コードグランプリ!秀逸な汚コードにまみれた解答臭をご覧あれ #逆リファクタリング
という記事を見かけたので、C++で遊んでみた。
本稿は2015年であり、qiitaにもそれっぽい記事が散見はするが、後学のために。
出題内容について
とりあえず記事より引用。
リファクタリングは外部から見た振る舞いを変えずに内部構造の保守性、可読性、拡張性などを改良する手法です。
今回は逆に、外部から見た目の振る舞いを変えずに、内部構造をあなたの考える汚いコードに逆リファクタリングしてください。・リファクタリング前 対象プログラム
print gets.upcase.reverse.split('').join(' ')
リファクタリング前のプログラムは Ruby ですが、以下の制約を満たしていれば、リファクタリング後のプログラムはRuby以外でも構いません。
提出可能な言語は、ideoneで動作確認可能な言語です。
- 制約1: 標準入力「CodeIQ」に対して、「Q I E D O C」を標準出力すれば正解とします。
※出力文字列の各文字の間には半角スペースがあります- 制約2: ideoneで実行不可能な場合は不正解とします。提出コードをそのままコピペすれば実行可能な状態にしてください。
- 制約3: プログラムは1,000文字以下にしてください。提出コードをRubyのsizeメソッドでチェックします。
まずC++で実装
本記事はrubyのコードがベースなので、とりあえずC++でかみ砕く。
ideoneはC++14にセットする。
# include <iostream>
# include <string>
# include <algorithm>
int main () {
std::string s;
std::cin >> s;
std::transform(s.cbegin(),s.cend(),s.begin(),toupper);
std::reverse(s.begin(),s.end());
auto sSize = s.size();
for (int i = 1; i < sSize * 2 - 1; i++) {
s.insert(s.begin() + i, ' ');
i++;
}
std::cout << s;
}
いわゆるupcaseはtoupper関数に文字列のイテレータを進めながら投げつけるtransform関数で、reverseはそのままイテレータをreverse関数で行う。どちらもalgorithmライブラリに入ってる。
スペースを随時差し込む仕様はinsertを文字倍でじわじわ進める形で行う。
C++erな人から見るとものすごくスパゲティに見えそうな、いかにもスクリプト言語(javascriptのごときキャメルケース変数名とか含めて)書いてた人上がりのようなコードであり、これでも十分きれいではないと思われる。
だがこれでは汚染度が足りないので、もっとC++らしい汚物に染めてみる。
あれと同じものを作る
C++ってなんでもできるけど作らないといけないんだよね。
というわけで、print gets.upcase.reverse.split('').join(' ')
みたいな記述のできるクラスを作って実装。
長めに見えるが、shift_jisで807バイトなので文字数制限は満たしている。
# include <iostream>
# include <string>
# include <algorithm>
class CodeIQ {
private:
std::string s;
public:
CodeIQ* getCin() {
std::cin >> this->s;
return this;
}
CodeIQ* upCase() {
std::transform(this->s.cbegin(),this->s.cend(),this->s.begin(),toupper);
return this;
}
CodeIQ* reverse() {
std::reverse(this->s.begin(),this->s.end());
return this;
}
CodeIQ* insertSpace() {
auto sSize = this->s.size();
for (int i = 1; i < sSize * 2 - 1; i++) {
this->s.insert(this->s.begin() + i, ' ');
i++;
}
return this;
}
std::string getResult() {
return this->s;
}
};
int main() {
auto c = new CodeIQ;
std::cout << c->getCin()->upCase()->reverse()->insertSpace()->getResult();
}
汚染ポイント
- すべては本来リファクタリング対象である
getCin()->upCase()->reverse()->insertSpace()->getResult();
- C++でメソッドチェーンを行うため、getResult以外は自身のポインタを返り値にし、return thisする。
- とにかくメソッドチェーン実装を優先したので各メソッド内は上記の実装のまま。
- とにかくメソッドチェーンしたかっただけで処理順を考慮していない。
- そのためinsertSpaceする前にreverseすると意図した処理ができない。
- メンバ変数sさえ定義できていればgetResultできるので、getResultではない。
- そもそも制約事項さえ満たせればいいので、原題のRubyメソッドと働きが違う。
- 生ポnewは放置。
率直な感想:
汚し足りない。せっかくC++14環境もらっているのに何かがもったいない。
なので。
ねえ、あの機能あったよね?
C++11とかC++14とかで追加されたあの機能を使う。
# include <iostream>
# include <string>
# include <algorithm>
int main() {
std::string s;
std::cin >> s;
auto f = [s]() mutable -> auto {
return [=](auto upS) mutable -> auto {
std::transform(upS.cbegin(),upS.cend(),upS.begin(),toupper);
return [](auto revS) mutable -> auto {
std::reverse(revS.begin(),revS.end());
return [](auto insS) mutable -> auto {
auto sSize = insS.size();
for (int i = 1; i < sSize * 2 - 1; i++) {
insS.insert(insS.begin() + i, ' ');
i++;
}
return insS;
}(revS);
}(upS);
}(s);
};
std::cout << f();
}
汚染ポイント
- ラムダを覚えたから使ってみたみたいなプログラム。C++の無名関数という魔境。
- ただ本家のほうでもRubyのlambda使いまくっていたので何番煎じだろう?
- auto使ってC++14から実装のジェネリックラムダにしている。
- 引数まで含めてautoできる部分全部変更済み。どんな型が動いているかわからん。
- ラムダ全部mutableにしてコピーしてきた自動変数弄りまくり。無計画。
- 入れ子? それっておいしいの?
- オプションで警告レベル上げるといろいろ表示されるけどWarningだし別にいい
- 変数名を略称にしてる点
- やっぱり処理自体はそのまんま
率直な感想
やっぱC++って糞だわ(誉め言葉)
試してみて
最近JavaScriptばかり触っていたため、たまにC++を弄ってみるといろいろと共通点やら相違点やらを確認できたりC++の重量級仕様にカルチャーショックがあったりなどしてよい気分転換になった。
やはりC++は変態言語である。学ぶならドMにならなければいけない。