web db+press リファクタリング

将来を想定した設計の罠
コードが劣化しないように将来を想定した設計をしないようにすること
なぜなら将来の想定は多くの場合、外れるから。

さらに仮に将来の設計が高確率でわかるとしても、やはり将来を見越した設計をするべきではない。
それは、現在不要になるコードを他のエンジニアが読まなくてはいけないからである。

そして一番問題なのは将来の設計を見越したコードが増えると「いつか使うから、このコードは消さない方がいい」となっていき、どんどん不要なコードを消さない流れになってしまう。
これをすると動いているコードと動いていないコードが出てきてしまい、負債がどんどん増えていく。

良いコードとはなにか。
それはチームの生産性を最大発揮させる事ができるコードである。
抽象度が高すぎても読めない人が多い独りよがりのコードになり、具体性が高すぎても冗長になる。

今のチームのレベル感に合わせて読みやすいコードが「良いコード」になる。

巷には~アーキテクチャが溢れている。今現在向き合っているコードを自分が知っている、〜アーキテクチャに適応させようとするのは危険である。
常にトレードオフは存在する。現状のコードがそのアーキテクチャとのマッチがいいのか、検証する必要がある。
そのためアーキテクチャの知識は多くあればあるほどよい。
100を知って10を現状のコードにアウトプットする。くらいでよい。

例え人のコードが凝集度、結合度に優れていなくてもそのアウトプットに対する評価は忘れないようにする。

凝集度とは関数の処理の役割の少なさを表す。
つまり、凝集度が高いほど役割が少ないということ。

凝集度が低いと一つの関数が色々な機能をもつので、再利用するのが困難になる

7つの凝集度
凝集度が低い

1. 偶発的凝集
2. 論理的凝集
3. 時間的凝集
4. 手続き的凝集
5. 通信的凝集
6. 逐次的凝集
7. 機能的凝集

凝集度が高い

偶発的凝集

無作為に作られた関数。特に凝集度などが考えられていない関数。

int add(int a, int b, bool e)
{
  e = true; ← 謎の処理
  a = a + b;
  return a;
}

ある関数において、その関数では必要のない処理が入ってしまってる時、凝集度は偶発的凝集となる。
これは後から見た人は何をやっているかわからないので、コードリーディングの工数がかさむ。
絶対にやってはいけない。修正後の例

int add(int a, int b, bool e)
{
 // 謎の処理を排除
  a = a + b;
  return a;
}

論理的凝集

関連のない処理をフラグによって切り替える関数
以下の例はflagでprintをするかaddをするかという処理を切り替えている。
関数名はprint_valueつまり何かしらの値を出力する関数でありそうなのに対して、flagで切り替わったら足し算をした値を返しているという一貫性がとれていない関数となっている。

int print_value(int a, int b, bool flag)
{
  if (flag)
  {
    printf("%d", a);
  }
  else
  {
    a = a + b;
  }
  return a;
}

上記の例は典型的な悪い例であるが、コードの中ではきれいに見える場合もある。
論理的凝集になっていないかのポイントはflagの値によって引数が不要な場合があるかどうかを見ることが大事です。
今回の場合だと引数bはflagがtrueの場合は使う必要がありません。
このような場合は関数を分けることができます。

論理的凝集は特に見分けるのが難しい(らしい)

時間的凝集

あるプログラミングを起動するときに、初期値に値を入れたりするケースが存在する。
例えば以下のようなサーバーを初期化する関数があるとする。このようにプログラムの動作のタイミングごとにまとめられている関数を時間的凝集という。

func serverInit() (*sql.DB, *Config)
{
  db, err := sql.Open("mysql", "root:@/my_database")
  if err != nil {
    log.Fatal(pingErr)
  }

  pingErr := db.Ping()
  if pingErr != nil {
    log.Fatal(pingErr)
  }

  config := &Config {
    APIKey: os.Getenv("API_KEY"),
    APISecret: os.Getenv("API_SECRET")
  }
  return db, config
}

↑のような関数の場合、例えばconfigの初期化を他の箇所でも行いたい時、sqlをopenするという関係のない関数まで呼び出さなければいけないということです。
コードが機能ではなく、時間でまとめられているのでこのようなことがおきます。
改善方法としては、各実装を機能関数に切り出します

func serverInit() (*sql.DB, *Config)
{
  db, err := connectDB()
  if err != nil {
    log.Fatal(pingErr)
  }

  config, err := loadConfig()
  if err != nil {
    log.Fatal(pingErr)
  }
  return db, config
}

このように、各機能ごとに実装を関数に切り出すことで凝集度を上げることができます。
ただし、このリファクタリングを行うかどうかは程度によります。
というのも、まずserverInit()は実装量が少ないです。
そのため、わざわざ機能ごとに関数を分けることで関数全体の見通しが悪くなってしまうケースがあります。
また、configを初期化するタイミングが他にない場合などは、わざわざ関数に切り出さなくても良さそうです。
時間的凝集におけるリファクタリングもケースバイケースだということがわかります。

手続き的凝集

手続き的凝集とは時間的凝集と近い内容で、機能的には関連がない内容が時間的観点でまとめられている。ただし、その順序が重要である。

例えばファイルへの書き込みである。ファイルへの書き込みは以下の手順で行われる。

1. ファイルのopen
2. ファイルへの書き込み

fd = open("file.txt", O_CRATE)
if (fd == -1)
  return ;
stat = write(fd, "hello", 6);
if (stat == -1)
  perror("write error");

しかし、ファイルopenとファイルへの書き込みは機能的には別の機能となる。
そのためこの２つの機能が一つの関数に記載されていると、再利用ができなくなってしまう。

改善方法としては、各機能を関数毎に分けることです。
しかしこれも程度によります。

通信的凝集

通信的凝集も時間的凝集と同じようなものです。しかし違う点は同じ値にて処理をしているという点です。
なので同一時間に同一値に対しての機能が集まった関数ということです。
例が単純で具体的な再現が難しいかもしれないです。
以下の関数においてchange3においても、関数の引数aが渡されているという状態です。

func (int a) string
{
  a = change1(a)
  a = change2(a)
  a = change3(a)
}

時間的な凝集なので、機能毎に分解できる可能性があります

逐次的凝集

通信的凝集の値の各関数が値の受け渡しをしているバージョンです

例えばある数字に対して以下のような加工を行うプログラムがあるとします。

1. 引数aを10で割った時の余りが0だった場合5を足す
2. 余りが0以外の場合は1を足す
3. それをダブルクオーテーションでくくって戻り値とする

func (int a) string
{
  if (a % 10 == 0)
    a += 5;
  else
    a += 1;

  string a_str = atoi(a);
  string ret = concat("\"", a_str, "\""); 
  return ret;
}

concatは結合をするオレオレ関数です。
このように同じ値に対しての処理をしているのですが、前半/中盤/後半で全くやっていることが異なります。
前半: 数値変換
中盤: 文字列変換
後半: 文字列結合

なので機能毎に分割する事ができます。

1. 数値変換
   2.文字列変換
   3.文字列結合

機能的凝集

各関数が機能毎にまとまっている状態
この状態が理想

第二章まとめ

リファクタリングするときの凝集度は7つに分けられる
この記事が参考になる
https://note.com/cyberz_cto/n/n26f535d6c575

偶発的凝集は必ず改善。
論理的凝集は可能な限り避けるべき。

時間的凝集
手続き的凝集
通信的凝集
逐次的凝集
この4つはケースバイケースになる。
またこれらは一つの時間的凝集としても問題ない

なので4つに分類できると考えるとわかりやすい

1. 偶発
2. 論理
3. 時間
4. 機能

また関数を分ければ分けるほど認知負荷が高くなるので、分ければよいというものではない。

凝集度という言葉は難しいので、役割として覚えたほうがよい気がする
少役割、多役割

結合度とは関数の独立性の低さ尺度
結合度が高いほど独立性が低く、結合度が低いほど独立性が高い

高独立、低独立
結合度が高いと他のデータや機能に影響をさせてしまうので良くない。

結合度も7つ存在する

1. 内容結合
2. 共通結合
3. 外部結合
4. 制御結合
5. スタンプ結合
6. データ結合
7. メッセージ結合

内容結合

ある関数に値を受け渡すときに引数を使うのではなく、メモリの番地をみて直接値を書き換えるという変数書き換え。

int main(){
  int *a = 0x00000008;
  *a = 10;
  twice();
  printf("a: %d", a);
}

int twice() {
  int *num = 0x00000008;
  *num = *num * 2;
}

まぁ実際こんな書き方はしないのでトリビア程度でよい

共通結合

global変数を使った結合 →共通結合は複数のglobal変数を使った結合なので後で直す

int num = 0;
int main(){
  num = 5;
  twice_ten();
  printf("num: %d", num);
}

void twice_ten() {
  num = 10;
  num = num * 2
}

グローバル変数を使うと、値が別の関数で書き換わっているかどうかの判断が難しいので結合度が高い
改善点としてはグローバル宣言→ローカル宣言に変更する