[S2226] Mutalel instance fields, that are only written to in the init() method, should be ignored

Clean Code Report False-positive / False-negative...
1

The rule S2226 finds mutable instance fields of Servlets and generally complains about them.

Fields should be ignored by this rule, if they’re only written to in the init() method.

I think, it is common practice to parse servlet parameters and collect information depending on them from within the Servlet’s init() method. It would be a pain, if the parameters would have to be parsed every time again every time you use them. At least I see no other way but to use mutable fields, because I cannot initialize them in the constructor.

3

Hi @mfroehlich,

Thanks for you feedback. Your suggestion does make sense, because according to the Servlet spec, there is no other way to initialize fields depending on the configuration than to do it in init() method.

Here is a ticket for it:

Regards,
Margarita

5

Hi Margarita,

that’s very cool. Thanks.

Cheers
Marvin

6

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.

7

Hey Marvin,

Thanks for your suggestion. I’m now back to fixing this FP and I cannot reproduce it at the moment. Could you please provide me with a reproducer, because it looks like we should support init() method assignments and the bug could be somewhere else.

Regards,
Margarita

8

This posing is a follow up for [S2226] Mutalel instance fields, that are only written to in the init() method, should be ignored
Please delete when merged. I cannot post a reply to that thread.

Here is my reproducer:

public class FooServlet extends HttpServlet
{
    private int foo = 0; // FP
    
    public int getFoo()
    {
        return foo;
    }
    
    @Override
    public void init()
    {
        this.foo = Integer.parseInt( getServletConfig().getInitParameter( "foo" ) );
    }
}
10

@mfroehlich post merged and original topic reopened :slight_smile:

11

Thanks @mfroehlich for the reproducer and sorry I forgot to open the thread :disappointed:
I now see the issue. We do not take into consideration init() method without parameters. Only the one having configuration as a parameter, which sounds wrong, because you still can access configuration in it. So I’ll update the ticket to track this method as well.

Regards,
Margarita