Better way to rewrite method without deprecated JRequest

Question

I believe JRequest is deprecated in the latest version of Joomla 3, have rewritten this function:

public function display($tpl = null)
{
    $app = JFactory::getApplication();
    $user = JFactory::getUser();
    
    $this->state = $this->get('State');
    $this->item = $this->get('Data');
    $this->params = $app->getParams('com_car');
    
    $db         =   JFactory::getDBO();
    $user       = JFactory::getUser();
    // Push a model into the view
    $model      = $this->getModel();
    $modelcar   = $this->getModel( 'product' );
    $id = JRequest::getVar('id','','default','int');
    $vid = JRequest::getVar('vid','','default','int');
    
    $prodDet = $modelcar->loadProduct($id,$vid);
    
    $this->assignRef('prodDet'  , $prodDet);
            
    $CatName = $modelcar->getCatName($id);
    $this->assignRef('CatName'  , $CatName);
}

New Code

public function display($tpl = null)
{
    $app = JFactory::getApplication();
    $user = JFactory::getUser();
    
    $this->state = $this->get('State');
    $this->item = $this->get('Data');
    $this->params = $app->getParams('com_car');
    
    $db         =   JFactory::getDBO();
    $user       = JFactory::getUser();
    // Push a model into the view
    $model      = $this->getModel();
    $modelcar   = $this->getModel( 'product' );
            
    $jinput = JFactory::getApplication()->input;
    $id = $jinput->get('id','','default','int');
    $vid = $jinput->get('vid','','default','int');
    
    //  $id = JRequest::getVar('id','','default','int');
    //  $vid = JRequest::getVar('vid','','default','int');
    
    $prodDet = $modelcar->loadProduct($id,$vid);
    
    $this->assignRef('prodDet'  , $prodDet);
            
    $CatName = $modelcar->getCatName($id);
    $this->assignRef('CatName'  , $CatName);

Is this the correct fix? Is there a better way to rewrite this method?

Answer 1

You could replace the use of JFactory with a Factory to prepare for Joomla 5.0 where it will be depreciated.

use Joomla\CMS\Factory;

class yourclassname
{
   public function display($tpl = null)
    {
    $app = Factory::getApplication()
    ...
    }
   ...
}

You could also see if the Application is already available to your class by checking if $this->app already exists.

As you have already got the Application with the above assigned to $app or $this->app then you don't need to get it again with $jinput = JFactory::getApplication()->input; , updating your code with these statements instead.

$id  = $app->input->get('id','','default','int');
$vid = $app->input->get('vid','','default','int');

or if $this->app is available.

$id  = $this->app->input->get('id','','default','int');
$vid = $this->app->input->get('vid','','default','int');

These two statements can also be updated to use Factory as well.

$db        = Factory::getDBO();
$user      = Factory::getUser();

And you have two statements for $user = JFactory::getUser();, you only need to get it once.