Should I refactor this code?

Posted by user156814 on Stack Overflow See other posts from Stack Overflow or by user156814
Published on 2010-04-13T22:02:00Z Indexed on 2010/04/13 22:13 UTC
Read the original article Hit count: 495

Filed under:
|
|

The code is for a view debate page. The code is supposed to determine whether or not to show an add reply form to the viewing user.

If the user is logged in, and the user is not the creator of the debate, then check if the user already replied to the debate.

If the user did not already reply to the debate then show the form... Otherwise, Check If the user wants to edit their already existing reply by looking in the url for the reply id

If any of these tests dont pass, Then I save the reason as an int and pass that to a switch statement in the view.

The logic seems easy enough, but my code seems a little sloppy.

Here's the code.. (using Kohana V2.3.4)

public function view($id = 0)
{
    $debate = ORM::factory('debate')->with('user')->with('category')->find($id);

    if ($debate->loaded == FALSE)
    {
        url::redirect();
    }

    // series of tests to show an add reply form
    if ($this->logged_in)
    {
        // is the viewer the creator?
        if ($this->user->id != $debate->user->id)
        {
            // has the user already replied?
            if (ORM::factory('reply')
                ->where(array('debate_id' => $id, 'user_id' => $this->user->id))
                ->count_all() == 0)
            {
                $form = $errors = array
                (
                    'body'      => '',
                    'choice_id' => '',
                    'add'       => ''
                );

                if ($post = $this->input->post())
                {
                    $reply = ORM::factory('reply');

                    // validate and insert the reply
                    if ($reply->add($post, TRUE))
                    {
                        url::redirect(url::current());
                    }

                    $form   = arr::overwrite($form,   $post->as_array());
                    $errors = arr::overwrite($errors, $post->errors('reply_errors'));
                }
            }
            // editing a reply?
            else if (($rid = (int) $this->input->get('edit'))
                    AND ($reply = ORM::factory('reply')
                                ->where(array('debate_id' => $id, 'user_id' => $this->user->id))
                                ->find($rid)))
            {
                $form = $errors = array
                (
                    'body'      => '',
                    'choice_id' => '',
                    'add'       => ''
                );

                // autocomplete the form
                $form = arr::overwrite($form, $reply->as_array());

                if ($post = $this->input->post())
                {
                    // validate and insert the reply
                    if ($reply->edit($post, TRUE))
                    {
                        url::redirect(url::current());
                    }

                    $form   = arr::overwrite($form,   $post->as_array());
                    $errors = arr::overwrite($errors, $post->errors('reply_errors'));
                }
            }
            else
            {
                // user already replied
                $reason = 3;
            }
        }
        else
        {
            // user started the debate
            $reason = 2;
        }
    }
    else
    {
        // user is not logged in.
        $reason = 1;
    }

    $limits = Kohana::config('app/debate.limits');
    $page   = (int) $this->input->get('page', 1);
    $offset = ($page > 0) ? ($page - 1) * $limits['replies'] : 0;

    $replies = ORM::factory('reply')->with('user')->with('choice')->where('replies.debate_id', $id);

    $this->template->title  = $debate->topic;
    $this->template->debate = $debate;
    $this->template->body   = View::factory('debate/view')
        ->set('debate',     $debate)
        ->set('replies',    $replies->find_all($limits['replies'], $offset))
        ->set('pagination', Pagination::factory(array
            (
                'style'          => 'digg',
                'items_per_page' => $limits['replies'],
                'query_string'   => 'page',
                'auto_hide'      => TRUE,
                'total_items'    => $total = $replies->count_last_query()
            ))
        )
        ->set('total', $total);

    // are we showing the add reply form?
    if (isset($form, $errors))
    {
        $this->template->body->add_reply_form = View::factory('reply/add_reply_form')
            ->set('debate', $debate)
            ->set('form',   $form)
            ->set('errors', $errors);
    }
    else
    {
        $this->template->body->reason = $reason;
    }
}

Heres the view, theres some logic in here that determines what message to show the user.

<!-- Add Reply Form -->
<?php if (isset($add_reply_form)): ?>

    <?php echo $add_reply_form; ?>

<?php else: ?>

    <?php
        switch ($reason)
        {
            case 1 :
                // not logged in, show a message
                $message = 'Add your ' . html::anchor('login?url=' . url::current(TRUE), '<b>vote</b>') . ' to this discussion';
                break;

            case 2 :
                // started the debate. dont show a message for that.
                $message = NULL;
                break;

            case 3:
                // already replied, show a message
                $message = 'You have already replied to this debate';
                break;

            default:
                // unknown reason. dont show a message
                $message = NULL;
                break;
        }
    ?>

    <?php echo app::show_message($message, 'h2'); ?>

<?php endif; ?>
<!-- End Add Reply Form -->

Should I refactor the add reply logic into another function or something.... It all works, it just seems real sloppy.

Thanks

© Stack Overflow or respective owner

Related posts about php

Related posts about refactoring