Pexels - challenge

Q1_reverse_string.rb

def reverse_string(input)
  return '' if input.nil? || input.size == 0
  rev_string = []
  input.split('').each_with_index{|w,i| rev_string.unshift(w) }
  rev_string.join
end

Q2_rails.rb

# 1- Write a short paragraph describing what is happening in the show action of PostsController (consider both the controller action and the view).

    # In the PostsController#show method we are return an instance variable @post. That variable can take the value of a Post object with the same ID that comes from the params and that has been published OR that variable will take the latest Post (the one with max ID, if any).
    

#2- How would you improve the existing code? Please show what changes you would make and describe why.
     
     # I'll post here my suggestions.
     
     # app/models/post.rb
     class Post < ApplicationRecord
       has_many :comments
       
       # I would create a scope to make the code cleaner
       scope :published, -> { where(published: true) }
     end

     # PostsController.rb
     class PostsController < ApplicationController
        def show
          # I would use the scope here. In terms of performance, there is no difference.
          # There is no need to use .first here since ID is (and should be) unique identifier.
          # I definitely would remove "id = #{params[:id]}" since it's not SQL injection proof.
          # Another way could be to use .find_by_id(params[:id]) to prevent an ActiveRecord exception in case there is no Post for the conditions. However, it requires to add a verification for @post in the view, before iterate.
          # I decide to use includes(comments: [:users]) to prevent N+1 queries in the view when iterating over each Comment.
          @post = Post.published.where('id = ?', params[:id]).includes(comments: [:users])
          
          # I would remove this last line, since it's not guarantee the Post.last has published = true 
          # @post ||= Post.last
        end
     end