Code logic in codeigniter

Can someone show me a optim and correct version of my code? I have a lot of if else switch, and I think my code is not organised

So I have a foreach in controller - method
I have get_user_details function in My Controller
The 3rd image is the view where I check if user is anonymous
The 4th image is same view another check if user is anonymous.

As you see I have a lot of if else and I don`t like my code.

In this line: $user = $this->users->select_details_by_id($user_id);
$user get all fields from database with details like:
$user->first_name would be Sebastian,
$user->last_name would be Vîrlan .

In controller method I have $un_questions->asker_id from database so if is 0 the user is anonymous.

Account.php

PHP Code:

public function questions() {

 $this->login_required();

         $header['title']  = 'Unanswered Questions';
         $data['page']   = DIR . __FUNCTION__;
 //page questions: user_id, status (1 = unanswered), limit, offset
 $unanswered_questions = $this->questions->select_by_profile($this->the_user->id, 1, 0, 30);

        foreach ($unanswered_questions as $un_questions) {

         //get asker_id and asked_id details
         $un_questions->asker = $this->get_user_details($un_questions->asker_id);

        }
        //number of questions
        $data['no_questions']   =  $this->questions->select_by_profile_and_status_count($this->the_user->id, 1);
                
 $data['questions_unanswered_block'] = $this->questions_unanswered_block($unanswered_questions, NULL);

 $this->load->view('header', $header);
        $this->load->view($data['page'], $data);
        $this->load->view('footer');
 } 


My Controller.php

PHP Code:

    public function get_user_details($user_id)
    {
        $user = new stdClass();
        
        switch ($user_id) {
            case 0:
                $user->name = $user->username = $user->link = $this->lang->line('anonymous');
                $user->city = "Unknown";
                $user->profile_picture = 'assets/images/default50.jpg';
                break;
            
            default:
                $user = $this->users->select_details_by_id($user_id);
                $user->name = $user->first_name.' '.$user->last_name;
                $user->link = '<a href="'.site_url('account/profile/' . $user->username).'">'.$user->first_name.' '.$user->last_name.'</a>';
                $user->profile_picture = 'uploads/users/'.md5($user->username.$user->id).'/'.$this->images->select_by_ids_image_and_type($user_id, 1);
                break;
        }

        return $user;

    } 


my_view.php

PHP Code:

<div class="profile-blog">

 <img class="rounded-x" src="<?php echo $q->asker->profile_picture ?>" alt="@<?php echo $q->asker->name; ?>">
    <div class="name-location">
 <ul class="list-inline pull-right">
 <li><small class="time-ago"><?php echo $this->time->ago($q->question_date); ?></small></li>
 <li><i class="question-delete fa fa-close"></i></li>
        
 
    </ul>
    <strong><?php echo $q->asker->link; ?></strong>
        <span>
            <i class="fa fa-map-marker"></i>
            <a href="#"><?php echo $q->asker->city; ?></a>
        </span>                
 </div>
 <div class="clearfix margin-bottom-20"></div>    
 <p><?php echo $q->question_text; ?></p>
 <?php if(isset($q->question_image) &&  $q->question_image): ?>
    <div class="row">
        <div class="col-sm-2 col-xs-4">
            <div class="thumbnail">
                <a class="fancybox-button zoomer" data-rel="fancybox-button" title="<?php echo $q->question_text; ?>" href="<?php echo base_url("{$q->image_path}{$q->question_image}"); ?>">
                    <span class="overlay-zoom">
                        <img class="img-responsive question-pic" src="<?php echo base_url("{$q->image_path}{$q->question_image}"); ?>" />
                        <span class="zoom-icon"></span>
                    </span>
                </a>
            </div>
        </div>
 </div>
 <?php endif; ?>
 <div class="panel panel-profile">
 </div>
</div> 


If you could copy/paste the code into the forum rather than using images, it would make it a lot easier not only to read the code, but to make changes and show them to you (if you use the PHP blocks, it will even give it some decent formatting). For example:

Code:

[php]
public function questions()
{
    // ...
}
[/php]

The ternary operator might help a little with the if/else conditions in your view, but it doesn't look like there's a simple solution there, other than using a different view if $q->asker_id == 0, or breaking up the view into partial views (and using different partial views for the 0 condition).

Bonfire

Practical CodeIgniter 3
CodeIgniter Testing Guide

You have two kinds of users, anonymous and not. These two users have separate needs. 

my quick suggestion would be: determine if the user is anonymous or not first.
And then have separate methods where appropriate so you aren't constantly having to check the same thing over and over.
Have separate methods in your controller that call the appropriate view. And then you can eliminate the logic checking in your views.

I have updated the code so I get rid of a if else.

So you said to have 2 methods . But if my page is http://website.me/index.php/account/questions and questions is one page where I show the questions from users and anonymous why to have 2 methods?

Here are 2 questions, one from user and one from anonymous.

You've removed most of the conditionals from the view, so the previous comments don't really make as much sense.

You should probably change your get_user_details() method in MY_Controller to be protected instead of public, since public methods in the controller are routable (unless they start with an underscore). So someone could visit /account/get_user_details/{$user_id}. It might not get them anything useful, but you never know.

Bonfire

Practical CodeIgniter 3
CodeIgniter Testing Guide

plus one to what mwhitney said.
also in your controller take a look at this line

Code:

$user = $this->users->select_details_by_id($user_id);

and then a bunch of stuff that depends on $user coming back.
the problem is -- what happens if the $user_id is invalid or nothing comes back?
you might need a fallback view to show if it fails, or you could make them an anonymous user.

another suggestion, all the code creating user->link, the picture etc - is in your controller. 
thats some of the first things to get refactored into models. so try and remove logic from views, 
and refactor view specific code from the controller into models. Then as you refactor your controllers become "thinner", 
and your views are mostly html and css. 

Wow mwhitney I would never guessed that I can call a my_controller function by url like controllers from controller folder. Thank you a lot for that tip ! When I start to call some functions from there few minutes ago I got a lot of errors. Now all are protected. Please tell me when should I use public?


Cartalot I did not understand this:

another suggestion, all the code creating user->link, the picture etc - is in your controller.
thats some of the first things to get refactored into models. so try and remove logic from views,
and refactor view specific code from the controller into models. Then as you refactor your controllers become "thinner",
and your views are mostly html and css.

I am new in codeigniter and mvc, can you show me a exaple of what you say there please?

so you would start out like you are doing and your get_user_details($user_id) method is in your controller.
but then as you refactor, basically that entire method could be refactored into a model, so then the controller
is just asking the model for $user and then passing $user to appropriate view.

the idea is that as you change specific details about the user link, or the profile picture, etc -
all of that detail is in a model method. the advantage is that your controller does not need to change at all.
AND if any other controllers need $user, you get it from the same model, instead of having to repeat the $user code in another controller.
AND again any changes to the $user method in the model - the other controllers are always getting the latest version.

Wow mwhitney I would never guessed that I can call a my_controller function by url like controllers from controller folder. Thank you a lot for that tip ! When I start to call some functions from there few minutes ago I got a lot of errors. Now all are protected. Please tell me when should I use public?

You might want to review the PHP manual's section on visibility.

• Public methods in a class can be called by other classes.
  
• Protected methods in a class can be called by classes which extend that class, but not by other classes.
  
• Private methods can only be called by the class which defined them.
  

In CodeIgniter, the Router translates the URI into a controller/method and calls that method. If the method isn't public, the Router can't call it. Since CodeIgniter pre-dates the visibility constraints, the router also honors an underscore prefix on the method name as an indication that a method can't be routed. This comes in handy for validation callbacks and other situations in which you need a public method in the controller, but don't want it to be accessed via the URI.

Since MY_Controller is typically the base controller for all of your other controllers, you usually don't need public methods or properties in MY_Controller. If you think you need a public method/property in MY_Controller, you probably should create a library for that method/property and any related functionality, then load it in the class which needs to call it, or load it in MY_Controller to configure the library, then access it via $this/get_instance()->some_library->some_method() in the class which needs to call it.

Bonfire

Practical CodeIgniter 3
CodeIgniter Testing Guide