2
\$\begingroup\$

I've been learning Ruby on Rails for the last few weeks. I've decided to do a portfolio website using Ruby on Rails to showcase all of my projects. Can you look through my code to see what you think?

I'm aware that one of the main principles of Ruby on Rails is DRY code so I have been trying to make my code as DRY as possible.

I have a welcome_controller which has 3 views/actions: index, skills and works.

I would love if you could review the "works" section of my website. The works view is divided up into four div sections (one for each work/project I am showcasing). The four div sections have the following ids:

  1. di_section
  2. virtual_realty_section
  3. todo_app_section
  4. alien_relatives_section

These four sections contain similar HTML so I have created partials for certain things like the introduction of the section etc. I have stored the data (e.g. the title and intro_paragraph) for these "intro" sections in an array called @section_intro_info which you can see in welcome_controller.rb.

My questions are as follows:

  1. Do you think my code could be even more DRY than it is? I could possibly store all of the data (i.e. bootstrap class names and ids) of each of the four sections in an array in the controller and then create a loop to generate the four sections. Or would that be going too far (in terms of trying to make it DRY)?
  2. Do you think I am following best practices or could I do anything better?

welcome_controller.rb

class WelcomeController < ApplicationController

    def index
        @section_intro_info = [{'section_title' => 'Sarah Monks', 
            'section_intro_paragraph' => 'Web Developer',
            'title_style' => 'title-light'}]
    end

    def skills
        @section_intro_info = [{'section_title' => 'My Skills', 
            'section_intro_paragraph' => 'I have a passion for web development and specialize in the following languages and frameworks.',
            'title_style' => 'title-dark'}]

        @skills = {'HTML5'=> 90, 'CSS3' => 92, 
            'JavaScript' => 85, 'jQuery' => 80, 'PHP' => 85, 
            'MySQL' => 60, 'Ruby on Rails' => 65, 'React JS' => 60, 'Angular JS' => 70, 'Bootstrap' => 70}
    end

    def works
        @section_intro_info = [{'section_title' => 'D I App', 
            'section_intro_paragraph' => 'I worked with a cross-functional team, planning, designing and building the following application... <br><br>The app is now available on both the Apple App Store and the Google Play Store.',
            'title_style' => 'title-light'},
            {'section_title' => 'V R', 
            'section_intro_paragraph' => 'I worked with a cross-functional team planning, designing and implementing a website to showcase an idea we had for a digital product we call "...".',
            'title_style' => 'title-dark'},
            {'section_title' => 'To do App', 
            'section_intro_paragraph' => 'I built a "To Do" list application using Angular JS, HTML5 and CSS3',
            'title_style' => 'title-light'},
            {'section_title' => 'Alien Relatives', 
            'section_intro_paragraph' => 'I built the following desktop application using HTML5,CSS3, JavaScript, PHP (OOP) and mySQL. 
                <br><br>The application is called "Alien relatives"...',
            'title_style' => 'title-dark'}
        ]
        @app_button_info = {'android' => 
                {'button_icon' => 'fa-android', 'store_name' => 'Play Store', 
                'store_link' => 'http://play.google.com/store/apps/...'}, 
                'apple' => 
                {'button_icon' => 'fa-apple', 'store_name' => 'App Store', 
                'store_link' => 'https://itunes.apple.com/ie/app/...'}}

        @di_app_screenshots = [{'img_title' => 'Home screen', 'img_src' => "image1.png"}, 
                    {'img_title' => 'Example title', 'img_src' => "image2.png"}, 
                    {'img_title' => 'Example title', 'img_src' => "image3.png"},
                    {'img_title' => 'Example title', 'img_src' => "image4.png"}, 
                    {'img_title' => 'Example title', 'img_src' => "image5.png"},
                    {'img_title' => 'Example title', 'img_src' => "image6.png"},
                    {'img_title' => 'Example title', 'img_src' => "image7.png"},
                    {'img_title' => 'User account Screen', 'img_src' => "image8.png"}]

        @showcase_image_data =  {'virtual_realty' => 
                        {'img_alt' => 'Virtual Realty', 'url' => 'http://www.my-domain.com/virtual_realty', 
                        'img_src' => '/assets/vr-showcase.png', 'width' => '666', 'height' => '516'},
                    'todo_app' => 
                        {'img_alt' => 'Todo App', 'url' => 'http://www.my-domain.com/todo', 
                        'img_src' => '/assets/todo-showcase.png', 'width' => '760', 'height' => '783'},
                    'alien_relatives' => 
                        {'img_alt' => 'Alien Relatives', 'url' => 'http://www.my-domain.com/alien_relatives', 
                        'img_src' => '/assets/alien_relatives_mac_narrow.jpg', 'width' => '1724', 'height' => '1408'}}

    end
end

works.html.erb

<div id="di_section" class="section_wrapper">
    <div class="overlay_black">
        <div class="section_content container-fluid">

            <%= render partial: 'partials/intro_section', :locals => {:section_intro_data => @section_intro_info[0]} %>

            <div class="row">
                <div class="col-md-8 col-md-offset-2 col-sm-8 col-sm-offset-2 text-center">
                        <!-- app store buttons -->
                        <%= render partial: 'partials/app_store_button', :locals => {:app_button_info => @app_button_info['apple']} %>
                        <%= render partial: 'partials/app_store_button', :locals => {:app_button_info => @app_button_info['android']} %>

                </div>
           </div>                           

         <div id="di_carousel" class="demo_carousel owl-carousel owl-theme row">
                <% @di_app_screenshots.each do |screenshot| %>
                    <div class="item">
                        <div class="thumbnail">
                            <%= image_tag("#{screenshot['img_src']}", size: "407x705", :alt => "#{screenshot['img_title']}", class: "img-responsive carousel_phone") %>

                            <div class="caption">
                                <h3><%= screenshot['img_title'] %></h3>
                            </div>
                        </div>
                    </div>
                <% end %>
            </div><!--/owl carousel -->

        </div><!--/container -->
    </div>
</div>

<div id="virtual_realty_section">
    <div class="container-fluid">
            <%= render partial: 'partials/intro_section', :locals => {:section_intro_data => @section_intro_info[1]} %>
             <div class="row">      
                <div class="col-md-offset-3 col-md-6 col-sm-offset-2 col-sm-8 col-xs-12">
                    <%= render partial: 'partials/showcase_image', :locals => {:showcase_image_data => @showcase_image_data['virtual_realty']} %>
                </div>      
            </div>              
    </div>
</div>

<div id="todo_app_section" class="section_wrapper">
    <div class="overlay_black_even">
        <div class="section_content container-fluid">
            <%= render partial: 'partials/intro_section', :locals => {:section_intro_data => @section_intro_info[2]} %>

            <div class="row">      
                <div class="col-md-offset-3 col-md-7 col-sm-offset-1 col-sm-10 col-xs-12">
                    <%= render partial: 'partials/showcase_image', :locals => {:showcase_image_data => @showcase_image_data['todo_app']} %>
                </div>      
            </div>  

        </div>
    </div>
</div>

<div id="alien_relatives_section">
        <div class="section_content container-fluid">
            <%= render partial: 'partials/intro_section', :locals => {:section_intro_data => @section_intro_info[3]} %>

            <div class="row">      
                <div class="col-md-offset-1 col-md-10 col-sm-12 col-xs-12">

                    <%= render partial: 'partials/showcase_image', :locals => {:showcase_image_data => @showcase_image_data['alien_relatives']} %>
                </div>      
            </div>  
        </div>
</div>

_intro_section.html.erb

<div class="row">
    <div class="col-md-offset-2 col-md-8 col-sm-offset-1 col-sm-10 col-xs-12">
        <div class="<%= section_intro_data['title_style'] %> wow fadeInDown intro-header">
            <h2><%= section_intro_data['section_title'] %></h2>
        </div>

        <p class="sub-head-light wow fadeInUp ">
           <%= section_intro_data['section_intro_paragraph'].html_safe %>
        </p>

    </div>
</div>

_app_store_button.html.erb

<div class="col-md-6 col-sm-6 col-xs-12 col-xxs-12">
    <%= link_to raw("<i class='fa #{app_button_info['button_icon']}'></i>
        <span class='small'>Download on the</span>
        <span class='big'>#{app_button_info['store_name']}</span>"), 
        app_button_info['store_link'], 
        :target => "_blank", 
        class: 'btn btn-app-store' %>

</div>  

_showcase_image.html.erb

<a href="<%= showcase_image_data['url'] %>">
    <%= image_tag("#{showcase_image_data['img_src']}", 
        size: "#{showcase_image_data['width']}x#{showcase_image_data['height']}", 
        :alt => "#{showcase_image_data['img_alt']}", 
        class: "img-responsive") %>
</a>
\$\endgroup\$
  • 1
    \$\begingroup\$ your controllers are acting more like views here since you have the html text and css classes declared there. Rather move that to a view, or if you want to keep this kind of stuff out of the templates, move it to view helpers which are accessible from the views. Beyond controllers, models, or any specific part of MVC, whenever you find your methods are growing in size because of some logic which is not fundamental to the purpose of that component, move that stuff somewhere else. \$\endgroup\$ – max pleaner Jun 19 '17 at 5:27
  • \$\begingroup\$ @maxpleaner thanks for the good advice. Makes sense Any chance you could give an example of how I would use helpers in this case? Where would I store the data that i have currently in the controller (for the intro section) and how would it interact with the helper? Thanks \$\endgroup\$ – Sarah Jun 19 '17 at 11:59
  • 1
    \$\begingroup\$ Moving data to helper won't make controller less heavy - you just will move data to the other file that is included into controller. Just moving stuff somewhere else is not a solution. Using Rails for this kind of task is a total overkill. All you need is: templates, data (in your case it seems reasonable to store it in YAML) and something that stitches that together. For example Middleman or jekyll will do just fine. \$\endgroup\$ – Aleksey V Zapparov Jun 21 '17 at 14:36
  • \$\begingroup\$ @AlekseyVZapparov Thanks. but I thought helpers were only accessible from the view (not the controllers)?. And you mean don't use rails at all for my site? Well this is only one section of the site. I also have a contact page where i make good use of the model and controller aspect of rails and also a simple CRUD functionality within the website so I will stick with rails.. I understand what you mean though about this task being a bit overkill. I appreciate the feedback. thanks \$\endgroup\$ – Sarah Jun 24 '17 at 17:29
  • 1
    \$\begingroup\$ Well, if that's the only place with some kind of static data - I would simply create models for that, but DB-less (read data from YAML). Helpers are just modules. If you move some logic from your controller to a module that is later included (like in case of helpers) you are not changing anything in fact. Just moving stuff from one place to another. \$\endgroup\$ – Aleksey V Zapparov Jun 26 '17 at 20:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.