Bug or Bad comment?

[linovski]

in
src/templates/velocity/java/pertable/manager.java.vm

we can find

    /**
     * Associates the $beanClass object to the $importedClass object.
     *
     * @param bean the $beanClass object to use
     * @param beanToSet the $importedClass object to associate to the $beanClass
     * @return the associated $importedClass bean
     */
    //4.1 SET IMPORTED
    public $beanClass set${importedClass}($beanClass bean,$importedClass beanToSet)
    {
        bean.$impKey.getForeignColumn().getSetMethod()(beanToSet.$impKey.getGetMethod()());
        return bean;
    }

and produce something like this:

    /**
     * Associates the NoticiaBean object to the NoticiaactivaBean object.
     *
     * @param bean the NoticiaBean object to use
     * @param beanToSet the NoticiaactivaBean object to associate to the NoticiaBean
     * @return the associated NoticiaactivaBean bean
     */
    //4.1 SET IMPORTED
    public NoticiaBean setNoticiaactivaBean(NoticiaBean bean,NoticiaactivaBean beanToSet)
    {
        bean.setIdnoticia(beanToSet.getIdnoticia());
        return bean;
    }

In method name, we can read 'set Noticiaactiva', but we are setting 'NoticiaBean'.
In comment we can read 'return (...) NoticiaactivaBean bean', but we are returning 'NoticiaBean'.

I think, if i change this I am not doing anything wrong, but at this time, I ask my self:
'Is this a bug or I am not understanding the point at all?'

Could you help me please?

[linovski]

Ok, we have a bug here.

We are saying that the method return NoticiaactivaBean, but in practice it return NoticiaBean

/**
* Associates ...
(...)
* @return the associated NoticiaactivaBean bean

(...)
//4.1 SET IMPORTED
public NoticiaBean setNoticiaactivaBean (...)
(...)

Do you confirm?

[Alain Fagot Béarez]

I would not confirm that as a bug until we could understand what the original intention was.  I am maintaining this code only since June 2007 and with very few insight from former developers.  That's why I feel reluctant to mark as bug, things that I don't understand.

By reading the template and the generated code, I am imaginating a whole different world...

As far as I may interpret it, it seems to me that such a method is setting a value into the NoticiaBean instance ($beanClass) which is a reference to the imported object NoticiaactivaBean.  As such I would see only the @return part of the documentation as misleading.  What it really returns is the original bean (NoticiaBean) with its field set to the associated NoticiaactivaBean.

I would expect this method to neither return the NoticiaactivaBean (since this method is in the NoticiaManager class) nor the NoticiaBean (since we didn't change the reference to the original bean parameter).

Would you accept such a template ?

/**
* Associates the $importedClass object to the $beanClass object as its
* $impKey.getForeignColumn().getJavaName() field.
*
* @param bean the $beanClass object to use
* @param beanToSet the $importedClass object to associate to the $beanClass
*/
//4.1 SET IMPORTED
public void set${importedClass}($beanClass bean,$importedClass beanToSet)
{
bean.$impKey.getForeignColumn().getSetMethod()(beanToSet.$impKey.getGetMethod()());
}

[linovski]

hmm, still not so good.

By the method name, we can read set in importedClass.

So, at least here, I will use the template such as this:

    /**
     * Associates the $beanClass object to the $importedClass object.
     *
     * @param bean the $beanClass object to use
     * @param beanToSet the $importedClass object to associate to the $beanClass
     * @return the associated $importedClass bean
     */
    //4.1 SET IMPORTED
    //  public $beanClass set${importedClass}($beanClass bean,$importedClass beanToSet)
    public $importedClass set${importedClass}($beanClass bean,$importedClass beanToSet)
    {
    //   bean.$impKey.getForeignColumn().getSetMethod()(beanToSet.$impKey.getGetMethod()());
    //   return bean;
        beanToSet.$impKey.getForeignColumn().getSetMethod()(bean.$impKey.getGetMethod()());
        return beanToSet;
    }

It's more intuitive to me this way