137

I'm getting this warning on Sonar:

Hide Utility Class Constructor:

Utility classes should not have a public or default constructor

My class:

public class FilePathHelper {
    private static String resourcesPath;
    public static String getFilePath(HttpServletRequest request) {
        if(resourcesPath == null) {
            String serverpath = request.getSession()
                                       .getServletContext()
                                       .getRealPath("");
            resourcesPath = serverpath + "/WEB-INF/classes/";   
        }
        return resourcesPath;       
    }
}

I want solution to remove this warning on Sonar Qube.

ℛɑƒæĿᴿᴹᴿ
  • 4,983
  • 4
  • 38
  • 58
Oomph Fortuity
  • 5,710
  • 10
  • 44
  • 89
  • 2
    Best disable this useless Sonar warning. The private constructor adds code noise only to keep some arbitrary Sonar rule happy. See here how to disable a rule: https://sqa.stackexchange.com/questions/24734/how-to-deactivate-a-rule-in-sonarqube – john16384 Feb 17 '20 at 11:18

12 Answers12

246

If this class is only a utility class, you should make the class final and define a private constructor:

public final class FilePathHelper {
   private FilePathHelper() {
      //not called
   }
}

This prevents the default parameter-less constructor from being used elsewhere in your code.

Additionally, you can make the class final, so that it can't be extended in subclasses, which is a best practice for utility classes. Since you declared only a private constructor, other classes wouldn't be able to extend it anyway, but it is still a best practice to mark the class as final.

ℛɑƒæĿᴿᴹᴿ
  • 4,983
  • 4
  • 38
  • 58
RoflcoptrException
  • 51,941
  • 35
  • 152
  • 200
  • 11
    `final` is actually redundant: a class with nothing but private constructors is already effectively final. No harm done by marking it, though. – Marko Topolnik Jan 18 '13 at 12:23
  • Yes that's true. Added it to the answer. – RoflcoptrException Jan 18 '13 at 12:25
  • 19
    You **must** mark the class as `final` for Sonar to actually consider the violation solved. Just adding private constructor doesn't clear the violation. – Cebence Apr 23 '13 at 11:19
  • why not an abstract class ? then you don't have a constructor – peter Sep 25 '13 at 08:13
  • 2
    Because you want to make sure that the class is not subclassed. – RoflcoptrException Sep 25 '13 at 16:31
  • 4
    Is there some Sonar rule that I need to enable for this? I have made my utility class `final` and have a private constructor. The constructor is now marked as the "uncovered line" – Karthick S Aug 11 '14 at 07:15
  • 6
    Indeed, how are you supposed to test/cover such 'unreachable' code? – dokaspar Jun 11 '15 at 20:25
  • @dokaspar, you may be able to test with PowerMock. I've never tested a utility class like this with PowerMock, but it's definitely handy for mocking static and private methods. Not sure if it will effect coverage. – wsams Aug 26 '16 at 19:32
  • 14
    So we should add unnecessary code to our classes, just to make some analysis tools happy? Someone can extend or instantiate this class. So what? Why is this critical? Can cause some errors? Or why should I even use tools like PowerMock to test a never used private constructor, just to fulfill the requirements of Sonar and my code coverage tool? – Sergej Werfel Oct 26 '16 at 10:49
  • 4
    @SergejWerfel Old post, but I agree. I was wondering what compelling reason there actually is for making it final or adding a private constructor. If someone wants to instantiate my utility class, go ahead. I don't really care. I don't see any reason that this should be considered "best practice" other than someone said so. If someone can actually give a valid reason as to why this is best practice, I'd be willing to change my mind. – Christopher Schneider Jun 05 '17 at 15:25
  • @SergejWerfel: code smells like this are always about the *whole* project. Instantiating a utility class is almost always a mistake, because there's nothing useful that object can do that others can't. So we want to stop people from making that mistake. The easiest way to do that is to make the compiler enforce that. Now Sonar solves the meta-mistake of not "marking" a class as a utility class even though its clearly meant as one. Whether or not that specific rule is enforced in any given project is obviously a per-project decision, but it's far from pointless. – Joachim Sauer Jun 17 '20 at 09:39
  • @Joachim Sauer instantiating a utility class is a beginner mistake, and I've never seen anyone do it. Sonar should flag the incorrect code, which is instantiating such a class to call a static method (in fact, most IDE's already flag this with a warning "method should be accessed in a static way"). Instead Sonar chooses to flag perfectly fine code. I always turn this "major" issue off first chance I get. – john16384 Feb 12 '21 at 22:54
22

I don't know Sonar, but I suspect it's looking for a private constructor:

private FilePathHelper() {
    // No-op; won't be called
}

Otherwise the Java compiler will provide a public parameterless constructor, which you really don't want.

(You should also make the class final, although other classes wouldn't be able to extend it anyway due to it only having a private constructor.)

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
12

I use an enum with no instances

public enum MyUtils { 
    ; // no instances
    // class is final and the constructor is private

    public static int myUtilityMethod(int x) {
        return x * x;
    }
}

you can call this using

int y = MyUtils.myUtilityMethod(5); // returns 25.
Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
10

Best practice is to throw an error if the class is constructed.

Example:

/**
 * The Class FooUtilityService.
 */
final class FooUtilityService{

/**
* Instantiates a new FooUtilityService. Private to prevent instantiation
*/
private FooUtilityService() {

    // Throw an exception if this ever *is* called
    throw new AssertionError("Instantiating utility class.");
}
javaPlease42
  • 4,699
  • 7
  • 36
  • 65
  • 3
    Why is this best practice? What are we gaining here? There is stupidity at all levels, but adding these lines just to prevent instantiation (which is fairly harmless in these cases) seems overkill, considering the half a dozen lines of code noise this introduces to an otherwise clean class with say only constants in it. – john16384 Feb 17 '20 at 11:20
  • @john16384 This is "best practice" in terms of satisfying modern static code analyzers which the language creators would agree is proper. Whether this is "best practice" for you, in your job, etc. depends on how your organization views this kind of thing. If your boss puts a high priority on static code analysis and squelching warnings or even "info" then your argument should be directed there. In a large team environment, it may not be good to assume everyone working on the code knows not to instantiate any given class. This probably falls into the realm of "defensive coding". – Thomas Carlisle Dec 04 '20 at 17:43
  • 3
    @ThomasCarlisle the language creators didn't create those code analyzers; useless warnings are something that you can turn off (like the infamous missing serialVersionId warning when not using serialization anywhere in your project) -- instead the tool should warn at the use site when someone is instantiating this class only to get access to a constant; people complain Java is verbose, and satisfying arbitrary unproven rules pushed upon us by poorly tuned "default" profiles in code analyzers ain't helping. – john16384 Dec 05 '20 at 08:24
8

You can just use Lombok annotation to avoid unnecessary initialization.

Using @NoArgsConstructor with AccessLevel.PRIVATE as bellow:

@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class FilePathHelper {
   // your code 
}
ℛɑƒæĿᴿᴹᴿ
  • 4,983
  • 4
  • 38
  • 58
Roman Topalov
  • 81
  • 1
  • 3
4

I recommend just disabling this rule in Sonar, there is no real benefit of introducing a private constructor, just redundant characters in your codebase other people need to read and computer needs to store and process.

user431640
  • 61
  • 7
  • There is a benefit. It will prevent the accidental creation of a redundant instance of the class. – Stephen C Mar 23 '21 at 05:16
  • That's why I say "real benefit". I know about the "reasoning" behind this rule. But is that a real problem? This can not happen - somebody should write code to create the class, so that's not accidental. And if someone has created an instance he does not need, it's their problem. In short, this does not deserve to worry about, and does not justify the extra characters in your code base, in my opinion. – user431640 Mar 23 '21 at 14:39
  • It is worse than that. `u.method()` looks like an instance method. But if `u` is an instance of a `UtilityClass` and (naturally) `method` is static, then we have some misleading code. The **real** utility of a `private` constructor is that it absolutely prevents that. (Have I seen it? Yes!) Whose problem is it? The programmer *using* your class. And by extension you ... because you designed the utility class to be instantiable. – Stephen C Mar 23 '21 at 15:21
  • Sonar is not just for the benefit of the programmer writing the code. It is also for the benefit of other programmers who need to use the code that the first programmer wrote. – Stephen C Mar 23 '21 at 23:00
  • 3
    If you like that, enable the sonar rule and adhere it. I personally don't want those extra charactes in my code. And don't worry, that person who was creating the Utility class instances won't be stopped from creating problems by the private constructor. He can always introduce endless loops, memory leaks, factor code in a way that it is unmanageble, or simply burn down the office. – user431640 Mar 30 '21 at 01:29
3

Alternative using Lombok is use @UtilityClass annotation.

@UtilityClass was introduced as an experimental feature in Lombok v1.16.2:

If a class is annotated with @UtilityClass, the following things happen to it:

  • It is marked final.
  • If any constructors are declared in it, an error is generated.
    • Otherwise, a private no-args constructor is generated; it throws a UnsupportedOperationException.
  • All methods, inner classes, and fields in the class are marked static.

Overview:

A utility class is a class that is just a namespace for functions. No instances of it can exist, and all its members are static. For example, java.lang.Math and java.util.Collections are well known utility classes.

This annotation automatically turns the annotated class into one.

A utility class cannot be instantiated.

By marking your class with @UtilityClass, lombok will automatically generate a private constructor that throws an exception, flags as error any explicit constructors you add, and marks the class final.

If the class is an inner class, the class is also marked static.

All members of a utility class are automatically marked as static. Even fields and inner classes.

Example:

import lombok.experimental.UtilityClass;

@UtilityClass
public class FilePathHelper {

    private static String resourcesPath;

    public static String getFilePath(HttpServletRequest request) {
        if(resourcesPath == null) {
            ServletContext context = request.getSession().getServletContext();
            String serverpath = context.getRealPath("");               
            resourcesPath = serverpath + "/WEB-INF/classes/";   
        }
        return resourcesPath;       
    }
}

Reference from official documentation:

ℛɑƒæĿᴿᴹᴿ
  • 4,983
  • 4
  • 38
  • 58
0

Although using @UtilityClass annotation will show issue on sonarCube. So basic problem is "Java provide a default no-argument public constructor" for a class. now we have two solutions -

  1. Remove @UtilityClass and make it static final class with private constructor.
  2. Instead of using it as class, Use it as Enum . but -

When the problem in sonarQube then use - @SuppressWarnings("java:###")
"###" rule number.

  • This does not provide an answer to the question. Once you have sufficient [reputation](https://stackoverflow.com/help/whats-reputation) you will be able to [comment on any post](https://stackoverflow.com/help/privileges/comment); instead, [provide answers that don't require clarification from the asker](https://meta.stackexchange.com/questions/214173/why-do-i-need-50-reputation-to-comment-what-can-i-do-instead). - [From Review](/review/late-answers/33688266) – Michael Katt Jan 26 '23 at 11:51
-1

Add private constructor:

private FilePathHelper(){
    super();
}
dur
  • 15,689
  • 25
  • 79
  • 125
  • 10
    welcome to stackoverflow! unfortunately this answer does not add any new information to the already given answers. and it's always better to elaborate a little more about how and why the code solves the problem. – René Vogt May 04 '16 at 05:26
-1
public class LmsEmpWfhUtils {    
    private LmsEmpWfhUtils() 
    { 
    // prevents access default paramater-less constructor
    }
}

This prevents the default parameter-less constructor from being used elsewhere in your code.

JustBaron
  • 2,319
  • 7
  • 25
  • 37
-1

SonarQube documentation recommends adding static keyword to the class declaration.

That is, change public class FilePathHelper to public static class FilePathHelper.

Alternatively you can add a private or protected constructor.

public class FilePathHelper
{
    // private or protected constructor
    // because all public fields and methods are static
    private FilePathHelper() {
    }
}
pamcevoy
  • 1,136
  • 11
  • 15
-3

make the utility class final and add a private constructor

  • 2
    welcome to stackoverflow! unfortunately this answer does not add any new information to the already given answers. and it's always better to elaborate a little more about how and why the code solves the problem. – René Vogt May 4 '16 at 5:26 – Rann Lifshitz Apr 19 '18 at 00:38