up vote 2 down vote favorite

I have a list of friends, new freinds are added via a form. When you click on a friend you see all the information about him and there is an Edit button. When clicking it, I want a form to be displayed (the same as the form for adding) but already filled with the current information about the friend. It is working this way:

class FriendController extends Controller
{
    public function editDisplayAction($id, Request $request)
    {
        $em = $this->getDoctrine()->getEntityManager(); 
        $friend = $em->getRepository('EMMyFriendsBundle:Friend')->find($id);

        if (!$friend) 
        {
            throw $this->createNotFoundException('There is no such friend');
        }

        $edit_fr_form = $this->createForm(new FriendType(), $friend);

        if($request->getMethod() != 'POST')
        {
            return $this->render('EMMyFriendsBundle:Friend:edit.html.twig', array(
            'id'=>$id, 'friend'=>$friend, 'fr_form' => $edit_fr_form->createView()));
        }
    }

    /*
     * @Edits the friend with the current id
     */
    public function editAction($id, Request $request)
    {
        $em = $this->getDoctrine()->getEntityManager(); 
        $friend = $em->getRepository('EMMyFriendsBundle:Friend')->find($id);
        $edit_fr_form = $this->createForm(new FriendType(), $friend);

        //if($request->getMethod() == 'POST')
        {
            $edit_fr_form->bindRequest($request);
            if ($edit_fr_form->isValid())
            {
                $em->flush();
            }
            return $this->redirect($this->generateUrl('friend_id', array('id' => $id))); 
        }
    } 
}

and this is the template:

{% block body %}

    {{ parent() }}

    <div>

        <h2>Edit a friend</h2>

            <form class="register" action="{{ path('friend_edit', {'id': friend.id})}}" method="post" {{ form_enctype(fr_form) }}>
            {% form_theme fr_form 'form_table_layout.html.twig' %}

                <div class="error"> 
                {{ form_errors(fr_form) }}
                </div>
            {% for field in fr_form %}
                {{ form_row(field) }}
            {% endfor %}
            <div class="woo">    
            <input class="button" type="submit" value="Submit"/>
            </div>       
        </form>
    </div>   

{% endblock %}

Is there some way this to work properly, without having two actions in the controller? And isn't it unrational and not very optimized in my way? Each suggestion how to make it better will be appreciated!

share|improve this question

1 Answer

up vote 1 down vote accepted

That HTML looks like Jekyll, or something very similar... Just out of curiosity, what is that, twig?

Anyways... The first thing you can do is follow the "Don't Repeat Yourself" (DRY) Principle. Both of your methods have very similar contents and could benefit from sharing those resources. For instance, you could set up the $em, $friend, $edit_fr_form all in a constructor as class properties, then do any specific work in the proper methods.

Another improvement might be to work on your variables. The names should be more descriptive for one ($entityManager instead of $em). Another is that you could use more of them. Sometimes its helpful to have a variable, even if that variable isn't going to be reused, just to help with legibility. For instance, the return statement in your editDisplayAction() method is very cramped. If you were to abstract some of those parameters as variables before passing them to the render() method, it would be much more legible.

As for two different methods/actions, what is wrong with that? Controllers don't have to be limited to having just one. Is there some specific reason that you are trying to change it, or does it just seem wrong to you? The only way to separate this would be to create another controller and HTML form. But this is unnecessary. It is perfectly understandable to reuse a controller for similar functionality. It would be a waste to separate this into multiple controllers and forms, just to avoid having more than one action in a controller.

share|improve this answer
Yes, the template it's Twig.The controller seems a bit unoptimized for me, bacause there is a bit code duplication and to do one thing - edit something I use two similar actions, so that's the reason I asked if there is a way to improve them or somehow use only one action. Thank you for your answer! :) – Faery Aug 28 '12 at 6:03

Your Answer

 
or
required, but never shown
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.