if文の綺麗なコードの書き方を知りたい

[sumura80]

概要

現在、画像を表示できるアプリを作成しております。 ３枚まで画像を投稿できるようにしているのですが、綺麗なコードの書き方を教えていただけませんでしょうか？ よろしくお願いいたします。　

実現したいこと

２枚投稿したとしても、投稿した分だけ表示したいと思っています。 下記のif文ですが、綺麗にするために調べたのですがうまく行きませんでした。 （下記のコードは一応正しく動作しております。）

解決するために行ったこと

<% if @post.image? %> <%= image_tag @post.image_url %> <% end %>

<% if @post.image_2? %> <%= image_tag @post.image_2_url %> <% end %>

<% if @post.image_3? %> <%= image_tag @post.image_3_url %> <% end %>

[konchanxxx]

これはPostというモデルのインスタンスをリストで取得してそのイメージを出力する感じだと思うので controllerで

@posts = Post.all

とかで配列を取得してviewでは

<% @posts.find_each do |post| %>
  <%= image_tag post.image %>
<% end %>

とかになるのかなと思いました

もしひとつのモデルに複数の画像を設定できるみたいになっているならそれは設計がちょっとまずい気がしますね 本来なら中間テーブルとかを用意してpost - image は1対多で紐づくように設計しないといけないです

class Post
  has_many :images
end

みたいな感じで持っていてほしい

[konchanxxx]

ざっくり説明したのでわからないところあれば追加で質問お願いします:bow:

[konchanxxx]

@sumura80

[sumura80]

ご説明ありがとうございました。

一つのモデルに複数の画像を設定できるのは、好ましくないんですね。

現在は、画像を１枚登録できる様になっております。そこで、３枚まで登録できる様にしたく、ブランチを切って試しておりました。 現在のブランチのPostは下記の様になっており、image_2: string（２枚目）、image_3: string（３枚目）という風にマイグレーションしました。

app/models/post.rb
class Post < ApplicationRecord
    mount_uploader :image, ImageUploader
    mount_uploader :image_2, ImageUploader
    mount_uploader :image_3, ImageUploader

Post( id: integer, title: string, description: text, created_at: datetime, updated_at: datetime, image: string, user_id: integer, likes_count: integer, category_id: integer, title_jp: string, description_jp: type: string, image_2: string, image_3: string)

ここで、一つ問題がおきました。 マイグレーションした中に、「type: string」があるのですが、この"type"が予約語だったため、エラーが生じてしまいました。 できればこのマイグレーションを削除して、imageもやり直したいと考えております。

予約語でマイグレーションして、登録しようとした時のエラー↓

ActiveRecord::SubclassNotFound in PostsController#edit

The single-table inheritance mechanism failed to locate the subclass: 'Pain Relief'. This error is raised because the column 'type' is reserved for storing the class in case of inheritance. Please rename this column if you didn't intend it to be used for storing the inheritance class or overwrite Post.inheritance_column to use another column for that information.

このマイグレーションをrollbackしたのですが、エラーになってしまうのですが、どの様に対処すればいいのか正直わかりません。

Status   Migration ID    Migration Name
--------------------------------------------------
up     20190116100449  Add imagebox newcolumns to posts

mini:med_plus_bs4_updated $ rails db:rollback
== 20190116100449 AddImageboxNewcolumnsToPosts: reverting =====================
-- remove_column(:posts, :image_3_name, :string)
rails aborted!
StandardError: An error has occurred, this and all later migrations canceled:

今回のエラーは、ブランチを切ってそこで起きたので、そのブランチを削除してもいいのですが、他のブランチを作成しても今回のマイグレーションがついてきてしまうため同じカラムを追加できないというエラーになってしまいました。

ちなみに、下記も試しましたが同じエラーでした。

rails db:migrate:down VERSION=20190116100449

可能であれば、本番も考慮に入れてrollbackで前回分を取り消したいと思っています。 そのほかは、「bin/rake db:migrate:resetでデータを全て削除。」という手段しかないのかなと思っております。 ご教示いただけませんでしょうか？よろしくお願いいたします。

[konchanxxx]

こちら事象を正確に把握したいのでリポジトリとbranchの共有をお願いしたいです:bow:

[konchanxxx]

現在は、画像を１枚登録できる様になっております。そこで、３枚まで登録できる様にしたく、ブランチを切って試しておりました。

これはvalidationで担保すれば良いかなと思いました。カスタムバリデーションを設定してそこで件数チェックを行うなど。 数に合わせて _1, _2 みたいなattributeを作成するのはスケーリングしないのでアンチパターンだと思います:bow:

[sumura80]

複数の画像を登録するときは、数に合わせて _1, _2 ではダメなんですね。 imageカラムに配列で入れていくのがいいのかなとも思いました。

こちらGitでのレポジトリーです。 showpage_renobation ブランチで作業をしている状態です。

https://github.com/sumura80/med_plus_bs4_updated/tree/showpage_renobation

[konchanxxx]

複数の画像を登録するときは、数に合わせて _1, _2 ではダメなんですね。 imageカラムに配列で入れていくのがいいのかなとも思いました

カラム配列も基本的に使わない方が良いです。抽出処理とかで抽出しづらかったり正規化できないためです。

GitHubの方みてみます:bow:

[konchanxxx]

予約語でマイグレーションして、登録しようとした時のエラー↓

こちらtypeをattirbuteに含んだ状態での現状のコードをあげてもらっても良いでしょうか？ 今上がっている最新のものだとrollbackできるのでエラー事象が再現できないですmm

[sumura80]

コードのご確認ありがとうございました。 昨日お伺いした、rollbackの件ですが、一度masterからテスト用に新しくブランチを作成し、 そこで下記のサイトをもとに、マイグレーションファイルを削除しました。 https://joppot.info/2014/10/16/2104

そうしたら、更にエラーにはまってしまいました。 スニペットのエラーで、masterブランチでadd /commitしても、ブランチの変更ができなくなってしまいました。 （masterからテスト用に作成したブランチは削除しました） こんな状態では、どの様に対処すればよろしいでしょうか・・・？:困ったあ:

1.master 2.問題になっていた“showpage_renoabtion” 3.masterからテスト用に“showpage_renovation_2"を作成し、2（“showpage_renoabtion”）で問題になっていたマイグレーションファイルを削除し、ブランチも削除し、現在 「masterブランチ」でエラーが出ている状態です。

※マスターから他のブランチに行けない状態です。（git pushもできません）:scream:

$ git status
On branch master
Your branch is ahead of 'origin/master' by 1 commit.
  (use "git push" to publish your local commits)

You are currently rebasing.
  (all conflicts fixed: run "git rebase --continue")

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    modified:   ../schema.rb
no changes added to commit (use "git add" and/or "git commit -a")

$ git push origin master
Counting objects: 6, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 689 bytes | 689.00 KiB/s, done.
Total 6 (delta 4), reused 0 (delta 0)
remote: Resolving deltas: 100% (4/4), completed with 4 local objects.
To https://github.com/sumura80/med_plus_bs4_updated.git
   5fe9b6e..8097ac0  master -> master

$ git checkout showpage_renobation
error: Your local changes to the following files would be overwritten by checkout:
    db/schema.rb
Please commit your changes or stash them before you switch branches.
Aborting

[sumura80]

おはようございます。 昨日のrollbackができない件ですが、ローカルのmasterでもどうしようもなくなってしまっていたので、Gitから問題が起きる前にpushしていたmasterを使用することにしました。 お騒がせしてして申し訳ありませんでした。

[sumura80]

現在、postコントローラーのshowアクションで、画像を表示するアプリを作成しております。 今回、複数画像を登録する方法を伺いたいと思っております。 前回試した、「ひとつのモデルに複数の画像を設定できる」は今回は避ける設計にするつもりです。 現在は、画像を1つ追加できる様になっております。ですので、画像２と名前、画像３と名前といった様な保存と表示をしたいと考えております。postにより保存する枚数は違うので、ここでif文を使い、登録した分だけ表示するというものです。

前回は、下記の様なカラムをpostに追加して、複数画像を登録する様な設計にしてしまいました。 image_2: string, image_2_name: string, image_3: string image_3name: string

　　 現在のモデルとコントローラーは下記の様になっております。

Postモデル
class Post < ApplicationRecord
    mount_uploader :image, ImageUploader

id: integer
title: string
description: text
image: string（複数の画像を登録したい）

PostsController
def show
     @post = Post.find_by(id: params[:id])
end

今回自分が疑問に思っているは下記の通りです。:confused: 1.通常のcarrierwaveでは、添付できるボタンは一つのみなので、どの様に複数添付するのか。 "multiple: true"を下記の様に加えれば複数できるとは思います。

<%= image_tag @post.image.url,multiple: true, %>

2.imageモデルを新たに作成し、has_many:imagesを追加するのか。(自分は配列かと思っていたのでより良い方法が知りたいです。)

　 なお、このサイトの例はあまり好ましくないということでよろしいですか？「配列」を使っている様です。 http://arthurxxx.hatenablog.com/entry/2018/02/05/234638

ご多忙の折恐縮ですが、よろしくお願いいたします。:bow:

[konchanxxx]

マスターから他のブランチに行けない状態です。

modified: ../schema.rb と出力されているので単純にコミットされていないファイルがあるように見えます。

$ git add .
$ git stash

とかでstashに逃すとかすれば良いかなと思いました:bow:

[sumura80]

ありがとうございます。 こちらマスターからどうしようもなかったので、Gitからマスターを取得し直しました。

[konchanxxx]

こちらマスターからどうしようもなかったので、Gitからマスターを取得し直しました。

こちら了解です:bow: こちら正しくはGitHubでしょうか？

[sumura80]

はい。GitHubのマスターで、問題となるマイグレーションの前の段階でpushしていたため、ローカルにダウンロードしてやり直しております。

[konchanxxx]

前回試した、「ひとつのモデルに複数の画像を設定できる」は今回は避ける設計にするつもりです。 現在は、画像を1つ追加できる様になっております。ですので、画像２と名前、画像３と名前といった様な保存と表示をしたいと考えております。

一つのPostモデルに対して中間テーブルを作成してImageモデルを複数持つような構成が良いかなと思いました。 これが近いかなと思います。 https://qiita.com/shinichiro81/items/4edb8af4a64991897d5a

[konchanxxx]

現在のモデルとコントローラーは下記の様になっております。

こちらコードのスニペット（切れ端）だけだと分かりづらいのでPRをあげて欲しいです:bow:

[konchanxxx]

PR(Pull Request)は下記の説明が良いと思います https://qiita.com/samurairunner/items/7442521bce2d6ac9330b

[konchanxxx]

1.通常のcarrierwaveでは、添付できるボタンは一つのみなので、どの様に複数添付するのか。 "multiple: true"を下記の様に加えれば複数できるとは思います。

<%= image_tag @post.image.url,multiple: true, %>

こちらアップロードですか？image_tagだと表示になる気がしました。

[sumura80]

表示ですね！失礼いたしました。 _formとeditにmultiple trueが必要なのかなと思いましたが、まずは中間テーブルの作成をしてみます。

[konchanxxx]

2.imageモデルを新たに作成し、has_many:imagesを追加するのか。(自分は配列かと思っていたのでより良い方法が知りたいです。) なお、このサイトの例はあまり好ましくないということでよろしいですか？「配列」を使っている様です。 http://arthurxxx.hatenablog.com/entry/2018/02/05/234638

このサイトはとりあえず保存して複数アップロードできる手法を記載しているだけだと思います。基本的な流れは良いですがデータの持ち方がNGかなと思います。

product.image.each

とか書いていて本来は配列などリストが変数に格納されていることがわかるように複数系で下記のように書かないといけないので

product.images.each

この記事を書かれた方はあまりRubyのコードを書いたことがない人かなと思いました。

やり方としてはfile_fieldを利用して

# view
<%= f.file_field :images, multiple: true %>

# controller
params.require(:post).permit(:images)

とかすればviewからのデータは配列で受け取れるのでそれを元にインスタンスを生成して保存すれば良いかなと思います。

imageのフィールド名をcontentにしたら下記のようなコードになるかなと思います。

@images = params[:images]&.map do |image|
  post.images.build(content: image)
end

# 本来はbulkでトランザクション処理した方が良いですが1件ずつ保存するなら下記
@images&.each(&:save)

[sumura80]

データの持ち方がきもなんですね。 現在、PRしようとしたのですが、下記の様なエラーになってしまいます。 1.cloneでローカルに保存。 2.質問用のbranchを作成 3.データを作成し、add / commit 4.git push origin　質問用のbranch エラー発生です。 どこか違うところがございましたでしょうか？

git clone https://github.com/rexitorg/menta.git
Cloning into 'menta'...
remote: Enumerating objects: 27, done.
remote: Counting objects: 100% (27/27), done.
remote: Compressing objects: 100% (20/20), done.
remote: Total 27 (delta 4), reused 19 (delta 3), pack-reused 0
Unpacking objects: 100% (27/27), done.
mini:med_plus_lifeabroad susumu$ cd menta/

git checkout -b plural_images
Switched to a new branch 'plural_images'
mini:menta susumu$ git push origin plural_images
remote: Permission to rexitorg/menta.git denied to sumura80.
fatal: unable to access 'https://github.com/rexitorg/menta.git/': The requested URL returned error: 403
mini:menta susumu$ git branch
  master
* plural_images
mini:menta susumu$ git push origin plural_images
remote: Permission to rexitorg/menta.git denied to sumura80.
fatal: unable to access 'https://github.com/rexitorg/menta.git/': The requested URL returned error: 403
```

[konchanxxx]

どこか違うところがございましたでしょうか？

原因はわかったのですが、エラー解消のスキルアップのためにこちらのエラーに原因と思われる箇所を教えていただきたいです:bow:

[sumura80]

スキルアップの質問ありがとうございます！ 「 The requested URL returned error: 403」から推測するに、リモートのURLにユーザ名を含めてあげる必要があるということでしょうか？（下記実際例） git remote set-url origin https://sumura80@github.com/rexitorg/menta.git/

いつも自分のレポジトリにしかPR出していなかったので、初めての表示でした:smiley:

[konchanxxx]

惜しいのですが原因はこの部分ですね

remote: Permission to rexitorg/menta.git denied to sumura80.
fatal: unable to access 'https://github.com/rexitorg/menta.git/': The requested URL returned error: 403

意味的にはこのmentaというリポジトリに権限（permission）がないことを意味しています。 @sumura80 sanのアカウントをcollaboratorとしてこのリポジトリに登録していないためだと思います。

PRは自分のリポジトリのコードの変更を加える際に作るものなのでこのmentaのリポジトリではなく現在作成中のアプリケーションのリポジトリでPRを作成して頂ければと思います:bow:

[sumura80]

理解が遅くてすみません。 自分のGitHubレポジトリーでPRをだすということでしょうか？ 現在のブランチをお伝えすれば良いのでしょうか？ 自分のレポジトリーでPRを出しても、近藤先生が見れるのかなと思いました。:bow:

[konchanxxx]

自分のリポジトリでPRを作成してもらってそのリンクを添付してもらえればと思います:bow:

[sumura80]

承知しました。では、一旦コードを書いてみます。 そのあとリンクを添付いたします。 ご丁寧な説明ありがとうございます。:simple_smile:

[konchanxxx]

承知しました！ありがとうございます:smile:

[sumura80]

一つのform_forで複数のテーブルに保存する方法を模索中です。:confused: https://qiita.com/shinichiro81/items/4edb8af4a64991897d5a

今回改めて考えたのですが、添付した画像の様にしたいと思っております。 現在は、画像１が登録できる様になっております。 今から追加したい内容は、画像2と画像2の名前・画像3と画像3の名前を登録。 さらに、ある投稿によっては、コメントに説明を加えるコメント説明用画像1・コメント説明用画像2を登録できる様にしたいと思っております。こちらには、名前は必要ありません。

この様に上記のサイトの方法では、画像２に名前を追加したり、コメント説明画像をベットで追加できるのかちょっとわかりませんでした。 ご意見を聞かせていただければ幸いです。:bow:

[konchanxxx]

現在は、画像１が登録できる様になっております。 今から追加したい内容は、画像2と画像2の名前・画像3と画像3の名前を登録。 さらに、ある投稿によっては、コメントに説明を加えるコメント説明用画像1・コメント説明用画像2を登録できる様にしたいと思っております。こちらには、名前は必要ありません。

オブジェクトとしては下記が存在するのかなと思っています。

• 投稿
• 投稿画像
• コメント
• コメント画像

投稿画像とコメント画像は別々のテーブル、オブジェクトとして設計しても良いしちょっとレベルの高いことをするならポリモーフィックという手法を使うことで対応できるかなと思います。

https://qiita.com/itkrt2y/items/32ad1512fce1bf90c20b#%E3%83%9D%E3%83%AA%E3%83%A2%E3%83%BC%E3%83%95%E3%82%A3%E3%83%83%E3%82%AF%E3%81%AE%E8%AA%AC%E6%98%8E%E3%81%9D%E3%81%AE%E5%89%8D%E3%81%AB

またImageクラスに持たせる属性（attribute）は下記のようになると思っており、表示するかどうかはview側で制御すれば良いのかなと思います。

• 画像の名前
• 画像のURL
• 画像の説明

別々のテーブル、クラスとして定義するならERDとしては下記のようになるんじゃないかと思います。

[sumura80]

ご回答ありがとうございました。 画像を登録するモデルを2つ作ればいいということですね！その時に名前も保存できる様にしておけば確かに保存できますね。

ちょっと難しそうですが、もう一回四苦八苦してみます。:bow:

[sumura80]

今回、PRを作成しました。 このやり方で正しいでしょうか？

https://github.com/sumura80/med_plus_bs4_updated_master/pull/2

[sumura80]

@rexitorg 一応画像が登録できる様になりました。 ありがとうございました。

[konchanxxx]

確認します:bow:

[konchanxxx]

まず一番初めに目についたところ https://github.com/sumura80/med_plus_bs4_updated_master 配下に2つアプリケーションがあるのですが、こういう構成にはしないと思います。 例外もありますが基本的には1リポジトリ1アプリケーションの構成が良いと思います。

[sumura80]

そうなんです。 Githubからzipで持ってきて、pushしたらダブってしまい、どの様に削除すれば良いのかわからない状態でした:sweat_smile:

[sumura80]

pushやadd . / commitするたびにこの画像の様に全て出てきてしまい、その解決方法がわからずにいました。:scream:

[konchanxxx]

Githubからzipで持ってきて

基本SSHでcloneするのが良いかなと思います。zipは使わないです

.gitの管理ファイルの階層がずれているのでmvでディレクトリごと移動してgit initし直してremote branchの設定をするのが良いかなと思います:bow:

https://d.akiroom.com/2013-01/git-remote-add-after-git-init/

[konchanxxx]

ざっくりレビューしました:bow:

[konchanxxx]

コードスタイルをチェックするためにrubocopとか入れた方が良いかもですね:bow:

[sumura80]

レビューありがとうございました。 rubocopというのがあるんですね。確認してみます。

自分で運用することだけ考えていたので、ほかの人がわかる様にもしておくことが大事だというのがよくわかりました。

ありがとうございました。

[sumura80]

Githubからzipで持ってきて

基本SSHでcloneするのが良いかなと思います。zipは使わないです

.gitの管理ファイルの階層がずれているのでmvでディレクトリごと移動してgit initし直してremote branchの設定をするのが良いかなと思います🙇

https://d.akiroom.com/2013-01/git-remote-add-after-git-init/

ありがとうございます。 次回からcloneを使うように致します。 現在、添付画像の様な状態になってしまっているのですが、「mvでディレクトリごと移動」のmvはターミナルとかから行うのでしょうか？それともGitHub上でできるのですか？ 基礎的な質問で大変申し訳ありません:bow:

[konchanxxx]

こちら確認します:bow:

[konchanxxx]

自分で運用することだけ考えていたので、ほかの人がわかる様にもしておくことが大事だというのがよくわかりました。

このコードってどういった目的で作成しているものでしょうか？ポートフォリオとして使うようなものであれば当然他人が読めるような可読性の高い状態に保つ必要があるのかなと思います:bow: 自分で運用するサービスなだけの場合でも一般的にわかりやすい命名をするのが基本ですね。リーダブルコードとか読んで見た方が良いかもしれないです:bow:

[konchanxxx]

現在、添付画像の様な状態になってしまっているのですが、「mvでディレクトリごと移動」のmvはターミナルとかから行うのでしょうか？それともGitHub上でできるのですか？

GitHubではできないです。ターミナルです。

新しくリポジトリ作成しちゃった方が早いかもですね。その場合は下記のようになります

(1) GitHubでリポジトリを作成する (2) GitHubからSSHでクローンする (3) 移動したいアプリの.gitを削除する (4) アプリケーションをクローンしたディレクトリに格納する (5) ディレクトリのツリー構造は下記のようになる

[app_name_dir] <= リポジトリ名とアプリ名は合わせる
-app
-bin
..
.git

[sumura80]

ご丁寧な説明付きのご回答ありがとうございます。 こちらやってみます。 またリーダブルコードも購入してみます。 お忙しい中、ありがとうございました。:bow: