Размер видео: 1280 X 720853 X 480640 X 360
Показать панель управления
Автовоспроизведение
Автоповтор
4:04 推測ですが、この関数は 0 を渡したら0が返ってきた方が便利だと思います。売数もしくは売価が0なら売上0円が返った方が便利でしょう。nil が返ってきたら呼び出し側が nil チェックをする必要が生じます。つまり4,5行目は削除したい。逆に、入力としてnil が渡されていないかのチェックに書き換えるのであれば有用性はあります。
ざっくりまとめ・関数名はその中身の処理がわかる名前にする・変数名はその中身だけでなく型もわかる名前にする・余計な変数は作らない・マジックナンバーは使わない・エラー処理も書いておく・if文で「==true」の処理はいらないプログラミングは基本的にチームで行っていくものなのでただコードを書くだけじゃなくて伝わるコードを書くということが大事ですね。
@アップハンズ 全くもってその通りですね。コードを修正してコメントを修正し忘れたらつまらぬ事で悩む羽目に、、リーダブルコードはどんな言語を使う人でも必読書ですね。
最後の答えが「return文をつける」だと思ってしまって草
思った「これ比較しとるだけで何も返らんやん」って思ってしまったら違ったのかw
@@TL-ip3tl コードを省略したら、考えさせられるコードが出来上がるというのは「本当に良いコードなのかな？？」って最近思います。（型定義の省略とかも、どうなんですかね。。）下手したら、保守性の低下を招く可能性もあるかと。。（コーディングした人とメンテする人が、同程度のスキルなら大丈夫だとは思いますが）
この企画すごい良い 笑続編期待
2問目、dataオブジェクトを想定するんじゃなくて、引数をpriceとcountの2つにしたほうが処理の見通しがよく、再利用性が高い
ほんまこれ
7:12 if 6
Python以外でその比較ってできるの？
@@user-lj5dp2jj3d 逆にできない言語ってあるの？
@@anata_no_kansou (
pythonでは同じ条件を「if 6
左オペランドに変数というのは、リーダブルコード的にも合ってますよ。
素晴らしい企画でとても勉強になりました。是非、継続してほしい企画です！！
「優れたプログラムは、プログラムそのものが仕様書になる」の好例ですね。もちろん仕様書があることでプログラムの品質を保証するから仕様書は必須だと思います。
冗長性はわかりやすさとどう両立すればいいか、引っかかってたので勉強になりましたありがとうございます
今の小中学生がプログラミング勉強してるみたいだけどこういう初歩的なこと教えるべきだよなちょっとした英単語の意味覚えるのにも役立つし
いまの小中学生はブロックプログラミングというコードを書かないでゲームを作るというのをやっているところがほとんどです。自分はコードでやった方がおもしろいし多くのことができると思うんですがねぇ（by 現役中学生）
穴抜き問題やってるっていうのを見たわw
@@sirokuma5963まあプログラミング学習の目的は文法を覚える事ではなく、論理的な思考力、プログラミングの様にルールを基に秩序立てて考える力を育む事が目的のはずだからな。思考に重点を置いているんだろ
6:58 3問目は、ポリモーフィズムを利用するのはどうでしょうか？Rubyを知らないので、文法は適当で書きます。Bankインターフェースにtransferメソッドを定義して作成。NormalBankクラスとMizuhoBankクラスを作成。Bankインターフェースをそれぞれのクラスにインプリメンツし、transferメソッドに具体的な処理を実装。bank_transferメソッドの最初に、mapで[MIZUHO_BANK_CODE => MizuhoBankクラスのインスタンス, NORMAL_BANK_CODE => NormalBankクラスのインスタンス]みたいに定数とインスタンスのマッピングを作成しておく。あとは、処理実行時にmapから取り出したインスタンスを、Bank型の変数に代入し、変数.transfer()を実行。これで、どれだけバンクの種類が増えてもif文がないコードになりますがどうでしょうか？
↑のような考え方はどうでしょう？どなたか感想いただきたいです。
一般論としてはいいデザインだと思います。
@@mokemoke768 ご返信ありがとうございます。自信が持てました。ポリモーフィズムの使いこなしは難しいですが、うまく使えばきれいなコードになりますね。
自分もBankインターフェース使った方がいいと思いました
3問目is_mizuhoは無理に消さなくてもいいかなフラグの意図が明確になるし、デバッグログ出力したり、今後拡張するときに使い回したりする可能性もあるし
yagni
4:30の問題、そもそも入力が構造の時点で中身の計算が分からないから、priceとcount で入れた方がわかりやすい。後もっと言うなら、税率をかけるんだから、税込価格を計算するという関数名にすべき。
youtube_channel_existsに関しては、「channel」という名前のチャンネルが存在するか調べたいとき、Trueしか返って来ないと思うけど、本当に「channel」という名前のチャンネルがあるかはわからない。だから配列の中に完全一致する文字列が含まれているかどうかまで調べるようにしたらいいと思う。あとAPIのメソッド名まで直せるなら、get_channel_namesにしたほうがstr型の配列が返ってくることが分かって健全。
変数名って、複数人で開発する際は非常に大事になるんですね。個人で開発しているだけなので、全然気にしていませんでした。
12:40 初心者なので教えてほしいですが、"channel_list=....channels(channel_name); channel_list.count>0;"という書き方ですが、直接"....channels(channel_name).count>0;"はダメなんでしょうか？単にlistとして取得してからのほうが可読性の点でベターという感じでしょうか。
新卒の子にこれは見た方が良いとお薦めしてます。いつもためになる動画で勉強になります
今は人に見せるためでなく、５分後の自分へのプレゼントだと思いコメントやソースを修正してます。良い動画と感じました。結局、５分後の自分がつらいのですよね。。
1問目 あるあるです。最初作ろうと思ったことなんだけど、ここでできちゃうなって思って色々追加するとこうなる
bankのやつは通常処理なのかを判定するメソッドがあると、呼び出し側がコードに対する知識を持たないのでいいと思う
プログラミング初期のあるあるですよねー。特にフラグや演算子の過剰な使い方は、まだ頭がプログラミング言語ベースの論理思考に完全移行できていなく、どこか電卓を使って計算している最中の様な心理や思考癖が残っているからなんでしょうね。
3問目、コードを確かめる必要があるなら拡張性を意識してswitch文にするかもです。銀行のコードが1つだけとは思えないので。5問目、個人的にはchannelsの結果をnullチェックしたくなりますね。必要ない言語とか、絶対に最低でも空配列取得とかだったら良いんですが
三問目はそもそも user.bahk にexecute_transfer を生やしたいですね。bankがcodeをもってるなら内部で判別すれば良い気がします。
@@本山香-z5p いやいや、userが持ってるbank情報にexecute_transfer(振り込み実行)なんて機能持たせてはダメかと……
銀行コードが増える度に分岐が増えるのでメソッドの肥大化が免れない実装には変わりなさそうです可能であればFactory MethodパターンやStorategyパターンなどを用いて条件分岐を消す方向で実装したく思いました
@@Gent-Owl ちなみにその場合execute_transfer はどんなクラスに持たせますか？
@@本山香-z5p user.bankってクラスで表すとしたらユーザーそれぞれが持つ銀行の情報ってことで「UserBankInfo」とかになると思うんですよ。となればそれを呼び出して使う側、振り込みを実行する側、と考えると「Bank」クラスや「Atm」クラスのようなものではないですかね。そこはどんなシステムに乗せるのか次第でしょうか。
コメント欄も含めて勉強になる
3問目if user.is_studentはユーザーデータのis_studentカラムを参照しているのでは？だから下のis_student関数は関係ないと思ったんだけどね。関係あるのであれば、if is_student(user)と書かなければいけない。
社内でコーディング規約とかあるので、何が正解かはそれぞれですね。私の会社だと、•リターンに計算結果を直接返してはいけない。•引数で持ってるものは一旦ローカルに落とし込むなどなど、、Cメインなのでってのもありますが、、
C言語しか知らないけど、雰囲気で最後以外は正解できました。変数名や関数名を分かり易くするの大切ですね。続編見たいです。
とても勉強になりました。第二回開催お願いします
1問目のはwhereしてfirstするよりfind_byのほうが良さそうですupdateもupdate!にしてこのメソッド自体にも!を付けて例外を吐くメソッドって知らせると親切ですね
コードレビューを観れるのめっちゃありがたいです！勉強になりました！
前にdictionaryなのにxxListって命名して、先輩に騙したなって言われたの思い出した笑
問2、価格と個数0でもエラー起きなくないですか？Rubyだと起きるんですかね。TAX_RATEが少数なので切り捨てか切り上げも必要ですね。実際には消費税のみの表示も必要になるので、消費税計算関数を別途作ってこの関数内では税抜き累計に消費税を足すという処理のほうが後々楽です。あと個数0でエラーにするかどうかの判断は、この関数ではなく別の関数でやるべきでは。データオブジェクトのインターフェースで定義できてしまえば楽ですね。ついでに実際にはおまけ商品なんかもなくはないので、価格0は許容してもよさそう。クーポンの実装によっては価格マイナスもなくはないので0をエラー出すならマイナスも考慮しなくてはいけないような。
動画内では消費税だとは言ってないけど税率計算は厄介ですね。確かに消費税をかけてからクーポン100円とか値引きしてはいけないし、様々なキャッシュレス決済のポイント値引きやキャンペーンを考慮すると消費税計算は最後の最後にしなければならない。地代家賃に消費税はかからない。宗教法人やNPO法人の収入も消費税はかからない。消費税以外も考慮するとありとあらゆる税率計算は全て別々に関数を用意して掛ける順序も複雑。無料で客に渡すものでも在庫管理のためにレシートに0円表示が必要で在庫は帳簿上の資産として個数管理が必要ですね。
めっちゃ勉強になります！
2問目は関数内で値が0のチェックはしたくない派
1問目 0:30def update_user_paid_member_by_uid(uid) user = User.find_by(uid: uid) if(user.present?) return user.update(is_paid_number: true) // update!の方が良いかも // elseの処理も必要になりそう endend2問目 2:03TAX_RATE = 1.1def calc_amount_price(price, count)// dataで受け取るんじゃなくてpriceとcountに分けて受け取ったほうが良さそう return price * count * TAX_RATEend3問目 4:35MIZUHO_BANK_CODE = '0001'def bank_transfer(user) // userのbank, bankのcodeが必ずある前提 // ない場合もあるなら&.使うなりtry使うなり変数に代入してnilチェックするなり // なんでもかんでも.でつなげるのもよくない場合がある if user.bank.code == MIZUHO_BANK_CODE mizuho_bank_transfer(user) else normal_bank_transfer(user) endend4問目 7:08// 個人的には真偽値を返すならstudent?とかstudent_age?とか?で終わる関数名の方が好き// 2問目と同じく年齢を確認するだけならageで受け取るほうがいいかもdef is_student(user) return 6
最後の5問目seleniumとかでデータ引っ張ってくる時、findとfind_allで要素数とかちゃう時に変数名間違えたりしてたの思い出した
これ最高の企画じゃないですか💡
クソコードとかそういう言葉遣いが面白いと思ってる業界の雰囲気が怖い
小川さんに答えられる前に、停止して改善点を考える事ができて、面白い企画だった！最後の問題5のヒントとか、コード内にコメントで記載していただいてると助かったかなと思いました！
==trueは1番ナシな書き方。かと言って「！」演算子の有無で判断するのはあまり可読性高いとは言えない。1文字しか違わないから。個人的な最適解は「==false」のようにfalseの時だけ==をつける かな7文字(スペース込みだと9文字)多いか少ないかだと絶対に見分け着くから
素人です。ここで使われている言語は何という言語なのでしょうか？
まぁ要するにリーダブルコードを読めと言う結論ですね
三脚使わずにカメラマン立たせてるのなんかおもろい
8:40で消えたけど、修正前の11行目のインデントが揃ってなかった。
いつも楽しく動画見させていただいています。単純に疑問がありました。関数の引数のところが"data"とか"user"とか、多分オブジェクトが入ると思うんです。でもそれって外部から見てどんなクラスのオブジェクトを入れたらいいか分かりにくくないのでしょうか？要するに型がよくわからないといいますか…この思考はC言語系ユーザ特有かも知れませんが。
まじで助かります!
2回目もやって欲しいです！
楽しみながらコードレビューしてる、面白い企画だった。 次回企画があったら、これ関数にした方がよくない？ これクラス使ってよ＝みたいなコードレビューもみてみたいです。nil や data.price.zeroの.zeroは よく使われますか？ （.zeroは、普通の変数でもつかえるのかな？）はずかしながら初めてみました。
問2でcount=0でエラーって言ってるけど、除算じゃないからエラーにならないよね？
return を省略しているところとしていないところがあるけれど、何か理由があるのだろうか。
何言語か全く判らないのですが、２問目、今は１０％とか８％とかあるので、関数名calc_amount_price_with_tax(data,tax_rate)とかいかがでしょうか？
できれば *_price_after_tax とかがいい。普通に英語で after tax や before tax って言うから。
2問目の引数は dataはだめでしょ。品物を示す名前にしないといけない。 あと、型指定もしてあげないとだめ。
2問目の引数にあるdataは、priceとcountでそれぞれ受け取りたいなとも思いました。dataというデータ構造に依存しすぎている？かなぁ。まぁ好みの問題ですね。
勉強になりました
これは楽しい
この企画面白い！！
面白い動画でした。個人的には、問4でDISCOUNT_PRICEを定数で定義するのはアリなのかとか、問5でチャンネル数を見るためだけにDBアクセスが必要なのかとか、仕様部分が気になってしまいました。仕様設計も考慮すると、コードレビューがより複雑になりそうです。
めちゃためになる〜
2問目、1.1 は消費税ですがでは軽減税率の場合は0.8 なので、そもそも TAX をDI できた方が良いかなと思います。あと price と count を引数にとるようにする方が data とか言う謎のオブジェクトを作らないで済む後者に関しては TypeScript であればfunction calc(data: {price: number, count: number}): number のように実装しますが Python のシンタックスに明るくないための感想です関数名にしてもcalc → calc_amount_price は伸びただけでどのように使うのかがあまりピンとこないですね、やりたい事は税込価格の計算なので get_total_price などが妥当かなと思います(totalと言う変数を使うのはこのデータをGAなどに送る場合、大体税込価格のことをGAがtotalと呼ぶことからきています)
1問目、Userのis_paid_memberが冗長で、paidでいいんじゃないのと思いましたw 2問目、calc_amount_priceになにか副作用があるわけじゃないのだから動詞を使わず単に amount_price あるいは amount でよい気がします。4問目のreturnは、動画中で軽く触れられていますが全部省けます。これはポリシーにもよりますね。私なら全部省いちゃいます。以上、細かい点ですし全部ポリシーによることなので必ずしも間違っているとか良くないというわけではないです。こういうことが書かれた本って40年以上前のカーニハンの「プログラム書法」とかwriting solid codeくらいしか知らないけど、現代だともっといい本あるんすかね。
面白いから中級編もお願いします
プロマネ向けの内容ですね！
基本的にif elseでいずれもreturnするときはelseは不要。いつも下のように書いている。if (条件) return 値1endreturn 値2
その行数の節約いる？
アーリーリターンの亜種？バリデーションみたいに上から順に引っかかったらfalse返す、一連の前処理終わったら本処理を書くならありですが、ただ単に分岐でreturnするだけならreturn一行もしくはきちんとelseも囲む方がいい気がします。他の人が保守するときにはendの後ろにコード書けちゃう余地があるので混乱するかも。。
@@なななな-d5i プログラミング全体の話として、なるべくコード量を減らしたほうがミスをするリスクを減らせるし処理も早くなるので個人的にはこのほうがいいと思っています。保守をするときに関数の中を見て、if elseの中でreturnされているほうが、一瞬コード書けちゃう余地があるように見受けられませんか？関数の最後がifのendよりreturnで終わってるほうがスッキリして見えます。
@@nangakiya 確かにifの終わりで関数が終わっているのは私も好みではないです。ただ、真偽で別々の処理が入る場合はやはりelseで囲む方が見通しがよいように思います（if elseの2つの分岐であることが構造から明らかになるので）。実際には処理の目的や規模によって書き方を変えます。もちろん大規模になればコーディング規約作りますが、その時は結構悩みますね。。
@@なななな-d5i もう一点として、ifの中でreturnをしてる時点で、そのあとの処理はifの条件に引っかからなかった場合の処理ということがわかるので、わざわざelseを書く必要はないかと思いました。
1問目って、もう一点書くと、is_paid_memberがtrueの状態でuserデータ返ってこないよね。アップデートする前に取得したuserデータだから。そもそもこれは何の言語だろう、rubyかな。
ActiveRecord（ORマッパー）のupdateメソッドはオブジェクトの値を破壊的に変えちゃうのでuserが返ってきます
5問目のpython のdef の 関数宣言に ? を使えるのが気になりました。def youtube_channel_exist ？
PythonではなくてRubyです。Rubyでは使えます
@@mokemoke768 ありがとう。そうでしたか。勉強になります。
pythonのコード読むの苦手なんだよな
同じく…みんなが結構読みやすいのだと何になるだろ、C/C++かJavaあたり？
サムネの件、1.1ってなんだァｧ!!!!!(今から視聴)
4問目def get_price(user) if user.is_student return DISCOUNT_PRICE end return NORMAL_PLICEend
やべ、複数形にしとけば分かるやろってことで、listとかarrayに変数名ずっとしてなかった笑
他の言語でもぜひやってほしい
色々言いたいことあるけど、まず最初に何の言語かハッキリ説明した方がいい。今回の問題は、意図的に短いコードとして切り取ったのかもしれないけど、1〜2行程度で済むなら下手に関数を分割しない方が見やすいと思う。5問目なんか特に、書きかけのコードで続きを書き忘れたのかと思った。他に指摘するとすれば、1問目userとUserが混在してて紛らわしい。2問目値が適正かどうかは、dataに値を格納する段階でやっておくべきことだし、勝手に0を不正な値と決めつけるのはおかしい。(例：サービス品等で価格0になっている場合や、数量0を明示的に表示する必要がある場合)消費税率を掛けるなら、円未満の端数処理を何もしないのはおかしいし、一律10％でいいのか、8％の商品や非課税の商品が混在する可能性がないか、よく確認しとかないと後でフラグ判定の追加に悩んだり、商品データの構造の見直しが発生したりとか、修正が大変なことになる。4問目学生証の有無じゃなく年齢で判定してるのに、studentはおかしい。(特に大学生とか)多分、大人料金と子供料金の区別をしたいんだろうけど、5歳以下の幼児料金が定義されてなくてNOMAL_PRICEになるのはおかしい。無駄に関数を分割したせいでバグを見落とす典型例だと思う。あと、この言語の仕様詳しくないけど、本当にこの書き方でis_student呼び出せるのかな？
userは変数で、Userクラスのインスタンスのようなものが参照されています全体的に同意なのですが、初級者向けの動画なので大枠の指摘中心だと思いました（感想）
==trueはいらないけど、別に見にくくもないから残してる
よくわかんないけど、とある証明が効率的じゃないってこと？でも、証明ってアイデアの問題なわけで、効率問題にすべき？
9:10is_student内でuser.ageを使って学生かどうか判定してるけど学生かどうかと年齢は別なので、Flagとして別に持った方がいい情報かなと思ったりしました。日本でも30歳で大学入りなおす人とかいますし、学生かどうかの情報はtrue/falseで持ってるべきかなぁと。入力項目が増えてめんどくさくなるんですけどね。
この話いいね！
「Rubyなのかな？」とは思うんだけど…return user.age >= 6 and user.age
またやって欲しい
これってなんの言語なんですか？ruby？
(問2) 回答の括弧不要じゃないの?
Bルームなんすねー
なぜエンジニア系の動画はみんな、前提条件になる言語や環境の説明をしないのだろうか？
1.1ってそういうことか！
宗教色が強い動画やなあ
５歳以下は通常料金ｗ
rubyなんですかね？初めて見たけど、Pythonっぽいんですね。最後のはrubyではよくあるのかわからないけど、関数名に？があるのが違和感でした
これ言語なんですか？
モブプロやね
比較で6や18というり直値を使うのは良くないと思いますbooleanを返却するのに比較してtrue,falseを返すのは可読性という面ではありでクソコードは言い過ぎです
そもそもコメントが無いっていうのはダメですか！？
一般的にはコメントは無い方がいいね。するとすれば何が起きてるのかを説明するのではなく「なぜそうしたのか」を書くべきだし。
凄い共感できるけど、実際のコーディングで毎回ここまで意識出来てるかというと……(´・ω・`)というか、そもそもだけどある程度はコメント入れようぜって思う。
適切に命名したり、責務を切り分けてコードを書けば、読めば分かるコードに近づきます。読めば分かるコードに、わざわざコメントを書くのは冗長ですし、そもそも良いコードを書く努力をせずにコメントで解決しようとするのは姿勢としても良くありません。コメントは有用なものですが、基本的にコードの内容を補完する程度に留めておくべきだと思います。もちろん、上記はあるべき論ですし、組織の都合やプログラミング言語としての表現力が弱く、コメントがないとコードリーディングが難しいケースもあるかと思いますのでケースバイケースにはなりますが。
@@人間失格-c4z ケースバイケースなのはその通り。みんながみんな優秀なエンジニアならそれが最適なんだろうけど、仕事となるとコーディング初心者から毛が生えた程度の新人とかも見ることも考える必要がある。その辺りを考えると経験的にはある程度コメントは入れた方がプロジェクトとしてベターだと思うってのが言いたかった。
@@Hoshi-No-Chidori ああなるほど、まあ確かにあって損するものでもないですしね。確かにコメントの有無で言えばあった方が対応できるプロジェクトやメンバーは多そうです。
2:26 字幕「culc_price」ではなくて「calc_price」 にすべき ｗ 。次の「culc_amount_price」も同様😅9:54 is_student()が不要で 2行目に if(user.age >=6 and user.age
後者はダメですね。メソッド名の是非はさておき、年齢確認をするロジックが分散するので、Userインスタンスなどの特定のクラスやインスタンスにまとめたいです。（もしくは他の仕様クラスなど）
@@本山香-z5p 回答ありがとうございます。参考になりました。２箇所以上で「6歳以上18歳以下」を判定する箇所があることを想定してるのですね。
関心の分離・細分化的に、is_studentメソッドは欲しい。1度しか呼ばれないとかだったら直で書いてもいいと思うが、行数が少ない=素晴らしいコードではない。
@@ファギョ 回答ありがとうございます。実際のコードで２回以上呼ばれるなら 私もis_studentメソッド作りますね。 クイズ形式で１回しかでてこなかったのでチョットそこの判断がわからなかったです。「行数が少ない=素晴らしいコードではない」こちら覚えておきます。
@@Nijist5_BR この問題だと仕様(前提条件)が提示されていないので、関数化する必要ないんじゃね？って私もレビューで言っちゃうと思いますw
2問目①1.1がハードコーディングなのでよくない。定数宣言しといて、定数モジュールとかからもってくるとよさげでは？と思いました②関数の説明がない
声がヨビノリ
いんだよ動けばクソコードでも
これなんの言語ですか？無知です
Rubyという言語ですね。
最後のやつはexist = channels.count > 0return exits にしなさい
全体として引数のアノテーションがないのがやはり気持ち悪いw
4問目。returnがない！結果が返ってこない！！比較してるだけ！！そこは突っ込まない！？？ヤバイヨヤバイヨ～
Ruby では返ります
5歳以下って学割適用されないんだっけ？笑
PythonにはAny関数ってないん？countよりanyのが速いとか聞いたことがあるきがする
これ何の言語や？
4:04 推測ですが、この関数は 0 を渡したら0が返ってきた方が便利だと思います。売数もしくは売価が0なら売上0円が返った方が便利でしょう。nil が返ってきたら呼び出し側が nil チェックをする必要が生じます。つまり4,5行目は削除したい。
逆に、入力としてnil が渡されていないかのチェックに書き換えるのであれば有用性はあります。
ざっくりまとめ
・関数名はその中身の処理がわかる名前にする
・変数名はその中身だけでなく型もわかる名前にする
・余計な変数は作らない
・マジックナンバーは使わない
・エラー処理も書いておく
・if文で「==true」の処理はいらない
プログラミングは基本的にチームで行っていくものなので
ただコードを書くだけじゃなくて伝わるコードを書くということが大事ですね。
@アップハンズ 全くもってその通りですね。
コードを修正してコメントを修正し忘れたらつまらぬ事で悩む羽目に、、
リーダブルコードはどんな言語を使う人でも必読書ですね。
最後の答えが「return文をつける」だと思ってしまって草
思った
「これ比較しとるだけで何も返らんやん」って思ってしまったら違ったのかw
@@TL-ip3tl コードを省略したら、考えさせられるコードが出来上がるというのは
「本当に良いコードなのかな？？」って最近思います。
（型定義の省略とかも、どうなんですかね。。）
下手したら、保守性の低下を招く可能性もあるかと。。
（コーディングした人とメンテする人が、同程度のスキルなら大丈夫だとは思いますが）
この企画すごい良い 笑
続編期待
2問目、dataオブジェクトを想定するんじゃなくて、引数をpriceとcountの2つにしたほうが処理の見通しがよく、再利用性が高い
ほんまこれ
7:12
if 6
Python以外でその比較ってできるの？
@@user-lj5dp2jj3d 逆にできない言語ってあるの？
@@anata_no_kansou (
pythonでは同じ条件を「if 6
左オペランドに変数というのは、リーダブルコード的にも合ってますよ。
素晴らしい企画でとても勉強になりました。
是非、継続してほしい企画です！！
「優れたプログラムは、プログラムそのものが仕様書になる」の好例ですね。
もちろん仕様書があることでプログラムの品質を保証するから仕様書は必須だと思います。
冗長性はわかりやすさとどう両立すればいいか、引っかかってたので勉強になりました
ありがとうございます
今の小中学生がプログラミング勉強してるみたいだけどこういう初歩的なこと教えるべきだよな
ちょっとした英単語の意味覚えるのにも役立つし
いまの小中学生はブロックプログラミングというコードを書かないでゲームを作るというのをやっているところがほとんどです。自分はコードでやった方がおもしろいし多くのことができると思うんですがねぇ（by 現役中学生）
穴抜き問題やってるっていうのを見たわw
@@sirokuma5963まあプログラミング学習の目的は文法を覚える事ではなく、論理的な思考力、プログラミングの様にルールを基に秩序立てて考える力を育む事が目的のはずだからな。思考に重点を置いているんだろ
6:58 3問目は、ポリモーフィズムを利用するのはどうでしょうか？
Rubyを知らないので、文法は適当で書きます。
Bankインターフェースにtransferメソッドを定義して作成。
NormalBankクラスとMizuhoBankクラスを作成。
Bankインターフェースをそれぞれのクラスにインプリメンツし、transferメソッドに具体的な処理を実装。
bank_transferメソッドの最初に、mapで
[MIZUHO_BANK_CODE => MizuhoBankクラスのインスタンス, NORMAL_BANK_CODE => NormalBankクラスのインスタンス]
みたいに定数とインスタンスのマッピングを作成しておく。
あとは、処理実行時にmapから取り出したインスタンスを、Bank型の変数に代入し、変数.transfer()を実行。
これで、どれだけバンクの種類が増えてもif文がないコードになりますがどうでしょうか？
↑のような考え方はどうでしょう？どなたか感想いただきたいです。
一般論としてはいいデザインだと思います。
@@mokemoke768 ご返信ありがとうございます。自信が持てました。
ポリモーフィズムの使いこなしは難しいですが、うまく使えばきれいなコードになりますね。
自分もBankインターフェース使った方がいいと思いました
3問目
is_mizuhoは無理に消さなくてもいいかな
フラグの意図が明確になるし、デバッグログ出力したり、今後拡張するときに使い回したりする可能性もあるし
yagni
4:30の問題、そもそも入力が構造の時点で中身の計算が分からないから、priceとcount で入れた方がわかりやすい。
後もっと言うなら、税率をかけるんだから、税込価格を計算するという関数名にすべき。
youtube_channel_existsに関しては、「channel」という名前のチャンネルが存在するか調べたいとき、Trueしか返って来ないと思うけど、本当に「channel」という名前のチャンネルがあるかはわからない。だから配列の中に完全一致する文字列が含まれているかどうかまで調べるようにしたらいいと思う。
あとAPIのメソッド名まで直せるなら、get_channel_namesにしたほうがstr型の配列が返ってくることが分かって健全。
変数名って、複数人で開発する際は非常に大事になるんですね。
個人で開発しているだけなので、全然気にしていませんでした。
12:40 初心者なので教えてほしいですが、"channel_list=....channels(channel_name); channel_list.count>0;"という書き方ですが、直接"....channels(channel_name).count>0;"はダメなんでしょうか？単にlistとして取得してからのほうが可読性の点でベターという感じでしょうか。
新卒の子にこれは見た方が良いとお薦めしてます。
いつもためになる動画で勉強になります
今は人に見せるためでなく、５分後の自分へのプレゼントだと思いコメントやソースを修正してます。
良い動画と感じました。
結局、５分後の自分がつらいのですよね。。
1問目 あるあるです。
最初作ろうと思ったことなんだけど、
ここでできちゃうなって思って色々追加するとこうなる
bankのやつは通常処理なのかを判定するメソッドがあると、呼び出し側がコードに対する知識を持たないのでいいと思う
プログラミング初期のあるあるですよねー。特にフラグや演算子の過剰な使い方は、まだ頭がプログラミング言語ベースの
論理思考に完全移行できていなく、どこか電卓を使って計算している最中の様な心理や思考癖が残っているからなんでしょうね。
3問目、コードを確かめる必要があるなら拡張性を意識してswitch文にするかもです。銀行のコードが1つだけとは思えないので。
5問目、個人的にはchannelsの結果をnullチェックしたくなりますね。必要ない言語とか、絶対に最低でも空配列取得とかだったら良いんですが
三問目はそもそも user.bahk にexecute_transfer を生やしたいですね。
bankがcodeをもってるなら内部で判別すれば良い気がします。
@@本山香-z5p いやいや、userが持ってるbank情報にexecute_transfer(振り込み実行)なんて機能持たせてはダメかと……
銀行コードが増える度に分岐が増えるのでメソッドの肥大化が免れない実装には変わりなさそうです
可能であればFactory MethodパターンやStorategyパターンなどを用いて条件分岐を消す方向で実装したく思いました
@@Gent-Owl ちなみにその場合execute_transfer はどんなクラスに持たせますか？
@@本山香-z5p user.bankってクラスで表すとしたらユーザーそれぞれが持つ銀行の情報ってことで「UserBankInfo」とかになると思うんですよ。
となればそれを呼び出して使う側、振り込みを実行する側、と考えると「Bank」クラスや「Atm」クラスのようなものではないですかね。そこはどんなシステムに乗せるのか次第でしょうか。
コメント欄も含めて勉強になる
3問目
if user.is_studentはユーザーデータのis_studentカラムを参照しているのでは？だから下のis_student関数は関係ないと思ったんだけどね。関係あるのであれば、if is_student(user)と書かなければいけない。
社内でコーディング規約とかあるので、何が正解かはそれぞれですね。
私の会社だと、
•リターンに計算結果を直接返してはいけない。
•引数で持ってるものは一旦ローカルに落とし込むなどなど、、
Cメインなのでってのもありますが、、
C言語しか知らないけど、雰囲気で最後以外は正解できました。
変数名や関数名を分かり易くするの大切ですね。続編見たいです。
とても勉強になりました。
第二回開催お願いします
1問目のはwhereしてfirstするよりfind_byのほうが良さそうです
updateもupdate!にしてこのメソッド自体にも!を付けて例外を吐くメソッドって知らせると親切ですね
コードレビューを観れるのめっちゃありがたいです！勉強になりました！
前にdictionaryなのにxxListって命名して、
先輩に騙したなって言われたの思い出した笑
問2、価格と個数0でもエラー起きなくないですか？Rubyだと起きるんですかね。
TAX_RATEが少数なので切り捨てか切り上げも必要ですね。実際には消費税のみの表示も必要になるので、消費税計算関数を別途作ってこの関数内では税抜き累計に消費税を足すという処理のほうが後々楽です。
あと個数0でエラーにするかどうかの判断は、この関数ではなく別の関数でやるべきでは。データオブジェクトのインターフェースで定義できてしまえば楽ですね。
ついでに実際にはおまけ商品なんかもなくはないので、価格0は許容してもよさそう。
クーポンの実装によっては価格マイナスもなくはないので0をエラー出すならマイナスも考慮しなくてはいけないような。
動画内では消費税だとは言ってないけど税率計算は厄介ですね。
確かに消費税をかけてからクーポン100円とか値引きしてはいけないし、様々なキャッシュレス決済のポイント値引きやキャンペーンを考慮すると消費税計算は最後の最後にしなければならない。
地代家賃に消費税はかからない。
宗教法人やNPO法人の収入も消費税はかからない。
消費税以外も考慮するとありとあらゆる税率計算は全て別々に関数を用意して掛ける順序も複雑。
無料で客に渡すものでも在庫管理のためにレシートに0円表示が必要で在庫は帳簿上の資産として個数管理が必要ですね。
めっちゃ勉強になります！
2問目は関数内で値が0のチェックはしたくない派
1問目 0:30
def update_user_paid_member_by_uid(uid)
user = User.find_by(uid: uid)
if(user.present?)
return user.update(is_paid_number: true)
// update!の方が良いかも
// elseの処理も必要になりそう
end
end
2問目 2:03
TAX_RATE = 1.1
def calc_amount_price(price, count)
// dataで受け取るんじゃなくてpriceとcountに分けて受け取ったほうが良さそう
return price * count * TAX_RATE
end
3問目 4:35
MIZUHO_BANK_CODE = '0001'
def bank_transfer(user)
// userのbank, bankのcodeが必ずある前提
// ない場合もあるなら&.使うなりtry使うなり変数に代入してnilチェックするなり
// なんでもかんでも.でつなげるのもよくない場合がある
if user.bank.code == MIZUHO_BANK_CODE
mizuho_bank_transfer(user)
else
normal_bank_transfer(user)
end
end
4問目 7:08
// 個人的には真偽値を返すならstudent?とかstudent_age?とか?で終わる関数名の方が好き
// 2問目と同じく年齢を確認するだけならageで受け取るほうがいいかも
def is_student(user)
return 6
最後の5問目
seleniumとかでデータ引っ張ってくる時、
findとfind_allで要素数とかちゃう時に変数名間違えたりしてたの思い出した
これ最高の企画じゃないですか💡
クソコードとかそういう言葉遣いが面白いと思ってる業界の雰囲気が怖い
小川さんに答えられる前に、
停止して改善点を考える事ができて、面白い企画だった！
最後の問題5のヒントとか、
コード内にコメントで記載していただいてると助かったかなと思いました！
==trueは1番ナシな書き方。かと言って「！」演算子の有無で判断するのはあまり可読性高いとは言えない。1文字しか違わないから。
個人的な最適解は「==false」のようにfalseの時だけ==をつける かな
7文字(スペース込みだと9文字)多いか少ないかだと絶対に見分け着くから
素人です。
ここで使われている言語は何という言語なのでしょうか？
まぁ要するにリーダブルコードを読めと言う結論ですね
三脚使わずにカメラマン立たせてるのなんかおもろい
8:40で消えたけど、修正前の11行目のインデントが揃ってなかった。
いつも楽しく動画見させていただいています。
単純に疑問がありました。
関数の引数のところが"data"とか"user"とか、多分オブジェクトが入ると思うんです。
でもそれって外部から見てどんなクラスのオブジェクトを入れたらいいか分かりにくくないのでしょうか？
要するに型がよくわからないといいますか…この思考はC言語系ユーザ特有かも知れませんが。
まじで助かります!
2回目もやって欲しいです！
楽しみながらコードレビューしてる、面白い企画だった。
次回企画があったら、これ関数にした方がよくない？ これクラス使ってよ＝みたいなコードレビューもみてみたいです。
nil や data.price.zeroの.zeroは よく使われますか？ （.zeroは、普通の変数でもつかえるのかな？）
はずかしながら初めてみました。
問2でcount=0でエラーって言ってるけど、除算じゃないからエラーにならないよね？
return を省略しているところとしていないところがあるけれど、何か理由があるのだろうか。
何言語か全く判らないのですが、
２問目、今は１０％とか８％とかあるので、関数名calc_amount_price_with_tax(data,tax_rate)とかいかがでしょうか？
できれば *_price_after_tax とかがいい。普通に英語で after tax や before tax って言うから。
2問目の引数は dataはだめでしょ。品物を示す名前にしないといけない。 あと、型指定もしてあげないとだめ。
2問目の引数にあるdataは、priceとcountでそれぞれ受け取りたいなとも思いました。dataというデータ構造に依存しすぎている？かなぁ。まぁ好みの問題ですね。
勉強になりました
これは楽しい
この企画面白い！！
面白い動画でした。
個人的には、問4でDISCOUNT_PRICEを定数で定義するのはアリなのかとか、問5でチャンネル数を見るためだけにDBアクセスが必要なのかとか、仕様部分が気になってしまいました。
仕様設計も考慮すると、コードレビューがより複雑になりそうです。
めちゃためになる〜
2問目、1.1 は消費税ですがでは軽減税率の場合は0.8 なので、そもそも TAX をDI できた方が良いかなと思います。
あと price と count を引数にとるようにする方が data とか言う謎のオブジェクトを作らないで済む
後者に関しては TypeScript であれば
function calc(data: {price: number, count: number}): number のように実装しますが Python のシンタックスに明るくないための感想です
関数名にしてもcalc → calc_amount_price は伸びただけでどのように使うのかがあまりピンとこないですね、やりたい事は税込価格の計算なので get_total_price などが妥当かなと思います(totalと言う変数を使うのはこのデータをGAなどに送る場合、大体税込価格のことをGAがtotalと呼ぶことからきています)
1問目、Userのis_paid_memberが冗長で、paidでいいんじゃないのと思いましたw
2問目、calc_amount_priceになにか副作用があるわけじゃないのだから動詞を使わず単に amount_price あるいは amount でよい気がします。
4問目のreturnは、動画中で軽く触れられていますが全部省けます。これはポリシーにもよりますね。私なら全部省いちゃいます。
以上、細かい点ですし全部ポリシーによることなので必ずしも間違っているとか良くないというわけではないです。
こういうことが書かれた本って40年以上前のカーニハンの「プログラム書法」とかwriting solid codeくらいしか知らないけど、現代だともっといい本あるんすかね。
面白いから中級編もお願いします
プロマネ向けの内容ですね！
基本的にif elseでいずれもreturnするときはelseは不要。いつも下のように書いている。
if (条件)
return 値1
end
return 値2
その行数の節約いる？
アーリーリターンの亜種？
バリデーションみたいに上から順に引っかかったらfalse返す、一連の前処理終わったら本処理を書くならありですが、ただ単に分岐でreturnするだけならreturn一行もしくはきちんとelseも囲む方がいい気がします。
他の人が保守するときにはendの後ろにコード書けちゃう余地があるので混乱するかも。。
@@なななな-d5i プログラミング全体の話として、なるべくコード量を減らしたほうがミスをするリスクを減らせるし処理も早くなるので個人的にはこのほうがいいと思っています。保守をするときに関数の中を見て、if elseの中でreturnされているほうが、一瞬コード書けちゃう余地があるように見受けられませんか？関数の最後がifのendよりreturnで終わってるほうがスッキリして見えます。
@@nangakiya 確かにifの終わりで関数が終わっているのは私も好みではないです。ただ、真偽で別々の処理が入る場合はやはりelseで囲む方が見通しがよいように思います（if elseの2つの分岐であることが構造から明らかになるので）。実際には処理の目的や規模によって書き方を変えます。もちろん大規模になればコーディング規約作りますが、その時は結構悩みますね。。
@@なななな-d5i もう一点として、ifの中でreturnをしてる時点で、そのあとの処理はifの条件に引っかからなかった場合の処理ということがわかるので、わざわざelseを書く必要はないかと思いました。
1問目って、もう一点書くと、is_paid_memberがtrueの状態でuserデータ返ってこないよね。アップデートする前に取得したuserデータだから。そもそもこれは何の言語だろう、rubyかな。
ActiveRecord（ORマッパー）のupdateメソッドはオブジェクトの値を破壊的に変えちゃうのでuserが返ってきます
5問目のpython のdef の 関数宣言に ? を使えるのが気になりました。
def youtube_channel_exist ？
PythonではなくてRubyです。Rubyでは使えます
@@mokemoke768 ありがとう。そうでしたか。勉強になります。
pythonのコード読むの苦手なんだよな
同じく…
みんなが結構読みやすいのだと何になるだろ、C/C++かJavaあたり？
サムネの件、1.1ってなんだァｧ!!!!!
(今から視聴)
4問目
def get_price(user)
if user.is_student
return DISCOUNT_PRICE
end
return NORMAL_PLICE
end
やべ、複数形にしとけば分かるやろってことで、listとかarrayに変数名ずっとしてなかった笑
他の言語でもぜひやってほしい
色々言いたいことあるけど、まず最初に何の言語かハッキリ説明した方がいい。
今回の問題は、意図的に短いコードとして切り取ったのかもしれないけど、1〜2行程度で済むなら下手に関数を分割しない方が見やすいと思う。5問目なんか特に、書きかけのコードで続きを書き忘れたのかと思った。
他に指摘するとすれば、
1問目
userとUserが混在してて紛らわしい。
2問目
値が適正かどうかは、dataに値を格納する段階でやっておくべきことだし、勝手に0を不正な値と決めつけるのはおかしい。(例：サービス品等で価格0になっている場合や、数量0を明示的に表示する必要がある場合)
消費税率を掛けるなら、円未満の端数処理を何もしないのはおかしいし、一律10％でいいのか、8％の商品や非課税の商品が混在する可能性がないか、よく確認しとかないと後でフラグ判定の追加に悩んだり、商品データの構造の見直しが発生したりとか、修正が大変なことになる。
4問目
学生証の有無じゃなく年齢で判定してるのに、studentはおかしい。(特に大学生とか)
多分、大人料金と子供料金の区別をしたいんだろうけど、5歳以下の幼児料金が定義されてなくてNOMAL_PRICEになるのはおかしい。無駄に関数を分割したせいでバグを見落とす典型例だと思う。
あと、この言語の仕様詳しくないけど、本当にこの書き方でis_student呼び出せるのかな？
userは変数で、Userクラスのインスタンスのようなものが参照されています
全体的に同意なのですが、初級者向けの動画なので大枠の指摘中心だと思いました（感想）
==trueはいらないけど、別に見にくくもないから残してる
よくわかんないけど、とある証明が効率的じゃないってこと？
でも、証明ってアイデアの問題なわけで、効率問題にすべき？
9:10
is_student内でuser.ageを使って学生かどうか判定してるけど学生かどうかと年齢は別なので、Flagとして別に持った方がいい情報かなと思ったりしました。
日本でも30歳で大学入りなおす人とかいますし、学生かどうかの情報はtrue/falseで持ってるべきかなぁと。
入力項目が増えてめんどくさくなるんですけどね。
この話いいね！
「Rubyなのかな？」とは思うんだけど…
return user.age >= 6 and user.age
またやって欲しい
これってなんの言語なんですか？ruby？
(問2) 回答の括弧不要じゃないの?
Bルームなんすねー
なぜエンジニア系の動画はみんな、前提条件になる言語や環境の説明をしないのだろうか？
1.1ってそういうことか！
宗教色が強い動画やなあ
５歳以下は通常料金ｗ
rubyなんですかね？初めて見たけど、Pythonっぽいんですね。最後のはrubyではよくあるのかわからないけど、関数名に？があるのが違和感でした
これ言語なんですか？
モブプロやね
比較で6や18というり直値を使うのは良くないと思います
booleanを返却するのに比較してtrue,falseを返すのは可読性という面ではありでクソコードは言い過ぎです
そもそもコメントが無いっていうのはダメですか！？
一般的にはコメントは無い方がいいね。するとすれば何が起きてるのかを説明するのではなく「なぜそうしたのか」を書くべきだし。
凄い共感できるけど、実際のコーディングで毎回ここまで意識出来てるかというと……(´・ω・`)
というか、そもそもだけどある程度はコメント入れようぜって思う。
適切に命名したり、責務を切り分けてコードを書けば、読めば分かるコードに近づきます。
読めば分かるコードに、わざわざコメントを書くのは冗長ですし、そもそも良いコードを書く努力をせずにコメントで解決しようとするのは姿勢としても良くありません。
コメントは有用なものですが、基本的にコードの内容を補完する程度に留めておくべきだと思います。
もちろん、上記はあるべき論ですし、組織の都合やプログラミング言語としての表現力が弱く、コメントがないとコードリーディングが難しいケースもあるかと思いますのでケースバイケースにはなりますが。
@@人間失格-c4z ケースバイケースなのはその通り。みんながみんな優秀なエンジニアならそれが最適なんだろうけど、仕事となるとコーディング初心者から毛が生えた程度の新人とかも見ることも考える必要がある。その辺りを考えると経験的にはある程度コメントは入れた方がプロジェクトとしてベターだと思うってのが言いたかった。
@@Hoshi-No-Chidori ああなるほど、まあ確かにあって損するものでもないですしね。確かにコメントの有無で言えばあった方が対応できるプロジェクトやメンバーは多そうです。
2:26 字幕「culc_price」ではなくて「calc_price」 にすべき ｗ 。次の「culc_amount_price」も同様😅
9:54 is_student()が不要で 2行目に if(user.age >=6 and user.age
後者はダメですね。
メソッド名の是非はさておき、年齢確認をするロジックが分散するので、Userインスタンスなどの特定のクラスやインスタンスにまとめたいです。（もしくは他の仕様クラスなど）
@@本山香-z5p 回答ありがとうございます。参考になりました。２箇所以上で「6歳以上18歳以下」を判定する箇所があることを想定してるのですね。
関心の分離・細分化的に、is_studentメソッドは欲しい。1度しか呼ばれないとかだったら直で書いてもいいと思うが、行数が少ない=素晴らしいコードではない。
@@ファギョ 回答ありがとうございます。実際のコードで２回以上呼ばれるなら 私もis_studentメソッド作りますね。 クイズ形式で１回しかでてこなかったのでチョットそこの判断がわからなかったです。
「行数が少ない=素晴らしいコードではない」こちら覚えておきます。
@@Nijist5_BR この問題だと仕様(前提条件)が提示されていないので、関数化する必要ないんじゃね？って私もレビューで言っちゃうと思いますw
2問目
①1.1がハードコーディングなのでよくない。定数宣言しといて、定数モジュールとかからもってくるとよさげでは？と思いました
②関数の説明がない
声がヨビノリ
いんだよ動けばクソコードでも
これなんの言語ですか？無知です
Rubyという言語ですね。
最後のやつは
exist = channels.count > 0
return exits にしなさい
全体として引数のアノテーションがないのがやはり気持ち悪いw
4問目。returnがない！結果が返ってこない！！比較してるだけ！！そこは突っ込まない！？？ヤバイヨヤバイヨ～
Ruby では返ります
5歳以下って学割適用されないんだっけ？笑
PythonにはAny関数ってないん？countよりanyのが速いとか聞いたことがあるきがする
これ何の言語や？