すなのプログラミングノート

都内自社開発企業で最近は主にReact書いてます。日々の学びをアウトプットします。

GitHubでの3種類のマージの仕方

GitHub Git

GitHub上でマージの仕方が3種類あることを知り、それぞれどういう状態になるのかというのを学んだのでまとめる。

3つのマージ

普段何気なくマージしてたけど、実は方法は3つあった。
それがこちら。
右のタブを押すと出現する。

マージの方法3種類


普段はこの中の「create a merge commit」をやっていた。


マージ後の状態

ではそれぞれの方法でマージした後、一体どうなるのか。

説明するより図で見た方がきっと早い。
ということで作図した。


マージ後の状態


まず、状態の確認から。
masterからブランチを切ってY, Zとコミットが作られている。
その間にmasterはXというコミットが作られている。
コンフリクトはないものとする。


Create a merge commit

これがデフォルトで設定されているためよく見慣れている。
この場合、マージされた際にマージコミットMが生成される。
元のブランチの状態も全て残る。

この時のコミットメッセージはデフォルトだと「Merge pull request 
#{PR番号} from {ブランチ名}」となる。

Create a merge commit


元のブランチの状態もコミットログも全て残す正直者。


Squash and merge

この場合、マージコミットMは同様に生成されるものの、ブランチのそれまでの複数のコミットは一つのマージコミットにまとめられる。
かつ、元のブランチの情報は残らない。

マージコミットのコミットメッセージはデフォルトだと「{PRタイトル} (#{PR番号})」となる。

Squash and merge


元のブランチの状態は残さないが、コミットを一つにまとめるまとめ上手。


Rebase and merge

この場合、マージコミットは生成されない。
かつ、元のブランチの情報も残らない。

ブランチで作業していたにも関わらず、
あたかも「最初からmasterでコミットし続けてましたよ？」
と見えるような一番しれっとしたマージ方法。

Rebase and merge


元のブランチの状態は残さず、手柄を全て自分の物にするかのようなあくどいやつ。(あくまで個人のイメージです。)


設定で表示を変えられる

GitHubの「Settings > Options」から3種類のうち、どれを表示させられるか設定できる。

Merge方法設定


これで間違って別の方法でマージしちゃった。。。ということが無くなる。


どう使い分けるの？

元のブランチの状態を見たいなら「create a merge commit」一択。
元のブランチの情報が必要ないのなら、あとはコミットログを残すかどうかで
「Squash and merge」か「Rebase and merge」を決めるという感じかな。

ただ、結局のところチームで方針決めてそれに従うことになる。

個人的にはデフォルトのcreate a merge commitが一番好きだ。
ただこいつの弱点は、チーム開発でそれぞれがブランチ切ったりしてるとmasterブランチがかなり見づらくなること。

ちなみにうちのチームでは「Squash and merge」を採用している。
masterの状態が常に1プルリクに対して1コミットとなるのでこれが一番masterの状態がスッキリする。

個人的には「Rebase and merge」がいまいちメリットが分からない。。
masterのコミットの状態がどんどん増えるのに、情報少なくなるだけな気がする。。

ここは結構人によって好き好みが分かれる。
実際うちの開発チームでもブランチ情報残したい派とmasterスッキリさせたい派が半々くらいだった。
宗教戦争にならないように注意が必要。

Gitは色んな使い方があるからちょっとずつ覚えていきたい。

ブラケット記法の使い時

JavaScript React

Reactで文字列として受け取った引数を使って配列の操作をしようとしたら思うようにいかなかった。

こんな風に書いたらいいんだという、JSの記法を学んだので書き残し。
結論、ドット記法ではなくブラケット記法を使えばうまくいく。

前提

onMouseDownというイベントを2カ所で使っていて、それにイベントが発火するとonMouseDownHandler関数が呼び出される。
この時、2カ所のどちらでイベントが発火したかわかるように以下のようにtopかbottomかという引数を渡すことにした。

onMouseDown = {event => onMouseDownHandler(event, "top")};
...
onMouseDown = {event => onMouseDownHandler(event, "bottom")};


stateにはtopという配列とbottomという配列があって、引数として受け取った"top"または"bottom"に応じて、このどちらかのstateを操作したい。
ちなみにtopとbottomには配列の中に配列が入っているという形。

this.state = {
  top: [[]],
  bottom: [[]],
}


うまくいかなかった例

最初こんな風に実装した。
第二引数にtopまたはbottomが入ってきているので、それをtopOrBottomとして受け取り、それを使ってstateの中のtopかbottomかを指定する作戦。

onMouseDownHandler = (event, topOrBottom) => {
  this.state.topOrBottom[index.row] ... //index.rowは配列の行番号を指定
}


しかし、ここでエラー。
なぜかというと、topOrBottomは文字列("top"または"bottom")なのでstate.topOrBottomは受け付けてくれないらしい。
人の目で見るとここにtopかbottomが代入されるんだから同じじゃんと思うけど、プログラムは厳密だ。

うまくいった例

さて、じゃあどうすればいいのかというと、記法を変えればいい。
下記のようにするとうまくいった。

onMouseDownHandler = (event, topOrBottom) => {
  const topOrBottomState = this.state[topOrBottom]

  topOrBottom[index.row] ... //index.rowで配列の行番号を指定している
}


色々記述が増えているが、要はstate.topOrBottomの代わりにstate[topOrBottom]にしたってだけのこと。
(本当はスプレッド構文使ったけど、ここでは簡略化のため省略)


うまくいった理由

そもそもオブジェクトのプロパティにアクセスする方法にはドット記法とブラケット記法というのがある。

ドット記法

いつもよく使うstate.topのようなドットでつなげる方法

ブラケット記法

state["top"]のようにブラケットの中に文字列を使う方法


つまりブラケット記法だと文字列を使ってプロパティにアクセスすることができるようになる。

こんなのあったなーという感想。
ドット記法の方が楽だし、ブラケット記法なんていつ使うんだろうと思ってたけど、文字列を利用しなくてはならない場合にはこっちを使うしかない。

もう少し抽象化して言うと、今回のように変数を使って動的にプロパティにアクセスしたい場合はブラケット記法を使うしかない。

他にもドット記法だとプロパティ名で数字で始まるものは使えないけど、ブラケット記法なら使えるとかもあるみたい。
ただこれはあまり使う機会はなさそう。

基本はドット記法の方が読みやすいしこっちでいいけど、それで対応できない場合はブラケット記法が有効。
色んな書き方あるけど、それぞれそれなりに意味があったりするもんだね。

Reactはだいぶ慣れてきたものの、やっぱり生のJavaScriptの記述がまだまだ課題だなと認識する今日この頃。
少しずつ慣れていこう。

参考

「JavaScript」ドット記法とブラケット記法 - はらこメモ

letを使わない書き方

JavaScript React

ずっと放置してた技術ブログですが、業務でのインプット量があまりにも多く、アウトプットが追いついていないのでブログを再開することにしました。

Twitterで書いても流れるし、140文字じゃ収まらない。
しっかりまとめるほど労力もかけたくないので気軽にアウトプットしたい。

そんなレベルのものを書き連ねていきます。
自己満足感強めですが、それでもよければ見ていってください。

前置きはこれくらいにして早速内容に。

今回は業務でレビューを受けた際になるほどと思ったJavaScriptの書き方について。

結論、間違ってはいなかったけどlet使わないで書いた方がいいよというリファクタリング的なお話。

前提

  • コンポーネントのstateにshowSidebarというboolean型のstateがある。
  • これがtrueの時にsidebarのdivを表示させたい

疑似コードで書くとこんな感じ

renderSidebar = () => {
  //もしshowSidebarがfalseならば
  sideBar = null

  //もしshowSidebarがtrueならば
  sideBar = (
    <div>
      //sideBarの内容
    </div>
  )
}

...

return (
  <div>
    //maincontentsの内容
    {this.renderSidebar()}  //sideBarの呼び出し
    ...
  </div>
)

sideBar部分はrenderSidebarという関数で呼び出しておいて、showSidebarがtrueの時だけsideBarに内容が入るためそれが表示されるという仕組み

実際の実装

これを最初次のように実装していた。

renderSidebar = () => {
  const { showSidebar } = this.state;

  let sideBar = null;

  if (showSidebar) {
    sideBar = (
      <div>
        //sideBarの内容
      </div>
    );
  };

  return sideBar;
};

...

return (
  <div>
    //ここにmaincontentsの内容
    {this.renderSidebar()} //sideBarの呼び出し
    ...
  </div>
)


return内でrenderSidebar関数を呼び出し、showSidebarがtrueかfalseかで表示させる処理内容を変えるというもの。

ここでletを使った変数sideBarにnullを代入しておいて、showSidebarがtrueならdivを再代入してreturnしている。
ここをletを使わないで実装できるならその方がいいとのこと。

理由はこの2つ。

  • コードが見やすくなって簡潔になる
  • 予期せぬところで再代入が起こり、バグの原因になるのを避ける

2番目の理由が大きそう。
このレベルの処理なら大丈夫だけど、使わないにこしたことはないということなのだろう。

実際調べてみるとletはfor文の最初の宣言だけにすべきで、基本全部constにすべき的なのをよく見る。

後半は問題ないので、前半の関数の重要な部分だけ抜き出して書いた改良版がこちら

if (showSidebar) {
  return (
    <div>
      //sideBarの内容
    </div>
  );
};
  
return null;


そもそも変数すら使わずにtrueの場合にsideBarのdivをreturnする。
関数内でreturnされるのでその後の処理は行われない。

これによってtrueの場合は最初のif文が実行されてdivがreturnされる。
falseの場合はif文が実行されず文末のnullがreturnされる。
という処理になる。

確かに見やすくなった。

ここから先のリファクタリングはもはや宗教になりそうというお言葉と共に、さらなる改良版を教えていただいた。
nullを先に返した方が可読性が上がると思うとのこと。

それがこちら。

if (!showSidebar) null;
    
return (
  <div>
    //sideBarの内容
  </div>
);


うん、個人的にはこれが一番好きだ。
かなり見やすいし、スッキリしている。

showSidebarがfalseの時はnullを返してその後の処理は行わない。
先ほどのものと処理が逆になっただけだが、if文のネストがなくなっただけでもかなり見やすくなっていると思う。

Reactはコード量も多くなりがちなので、小さなことでもコードの可読性はこれからも意識していきたい。