0
\$\begingroup\$

I at one time used class forname newinstance but newinstance is deprecated. I wanted to be able to pass a class name as a string and be able to load a class, so I came up with:

    public static Object getClass(String myclass) throws ClassNotFoundException,
        NoSuchMethodException,
        InstantiationException,
        IllegalAccessException,
        IllegalArgumentException,
        InvocationTargetException {
    Class clz = Class.forName(myclass);
    return clz.getDeclaredConstructor().newInstance();

}

Which works good. And usage is:

    Jim.Newpager apager = (Jim.Newpager) Jim.Jiminject.getInstance().getClass("Jim.Newpager");

Does that look alright?

The whole class that does the inject is:

package Jim;

import java.lang.reflect.InvocationTargetException;
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;


public class Jiminject {

    private static final Jiminject instance
            = new Jiminject();

    private Jiminject() {

    }

    public static Jiminject getInstance() {
        return instance;
    }


    public static Object getClass(String myclass) throws ClassNotFoundException,
            NoSuchMethodException,
            InstantiationException,
            IllegalAccessException,
            IllegalArgumentException,
            InvocationTargetException {
        Class clz = Class.forName(myclass);
        return clz.getDeclaredConstructor().newInstance();

    }
}

I use for Tomcat, all works good.

\$\endgroup\$
4
  • 2
    \$\begingroup\$ Can you elaborate on why would you want to pass the class name as a string? The instantiating mechanism comes with a host of security issues and your example use case makes no sense as you already know the exact class you are instantiating. \$\endgroup\$ Nov 17 '21 at 6:07
  • \$\begingroup\$ That’s 33 lines of code , but the actual logic can be expressed concisely in one single line (and, notably, more concisely than your implementation). Why would the caller use your class rather than using Class.forName("Jim.Newpager").newInstance(); directly? Your class doesn’t even make usage easier — on the contrary. \$\endgroup\$ Nov 17 '21 at 11:33
  • \$\begingroup\$ .newInstance(); is deprecated. Don't you think I would want to continue using it if it wasn't deprecated. \$\endgroup\$ Nov 17 '21 at 17:34
  • \$\begingroup\$ @JimWhitaker Fair point, I should have used getConstructor().newInstance() instead of abbreviating. I still don’t see the point of wrapping this straightforward logic into a singleton whose verbose usage doesn’t make the calling code any more concise. \$\endgroup\$ Nov 18 '21 at 9:33
4
\$\begingroup\$

The concept is odd to me and I've made a comment about clarifications. In the mean time... a few pointers about code style.

The method name getClass suggests that you are getting a class, when in fact you are returning an instance. To better convey the purpose of the method, you should rename it to instantiateClass or newInstance etc.

The name prefx my (as in myclass parameter) is the de facto naming style when you can not bother to think about an actually good and useful name. Never use the "my" prefix. Ever. It is always a sign that you are skipping an important part of your work. In this case, just use className, because that's what the parameter represents.

The getClass method is a helper method that shortens the instantiation boiler plate. It would be helpful to the user if you also wrapped the exceptions into a single generic exception that signals failure to instantiate the class. They all result in the same action in you code, right? So no point in making the caller handle each case separately.

Since you are using the getClass method via the instance retrieved with getInstance() there is no point in getClass to be static. Either drop the static qualifier of delete the getInstance() method.

Package names are writte in lower case letters. So make it jim instead of Jim. Capitalized initial letter is reserved for classes so the "pearl necklace" Jim.Jiminject.getInstance makes me look for a class named Jim to get to the start of the code.

\$\endgroup\$
2
  • \$\begingroup\$ I will do the renaming. This is meant as a dependency injection class. The getDeclaredConstructor().newInstance(); comes direct from Oracle as the recommended way. The exceptions was an ide netbeans suggestion to add those in. I also have another with try catch. How would you do a DYI dependency injection. This is to get an instance of any needed class, not just the paginator, that's why I need to pass in a string. \$\endgroup\$ Nov 17 '21 at 17:41
  • \$\begingroup\$ I wouldn't do DIY dependency injection, I would use Spring or some other library. :) Or seriously, let's just say that there is not enough context for me to answer the question properly. Clearly you have a very specific use case that handles class names represented as strings. But I don't know enough about it to say anything useful. \$\endgroup\$ Nov 18 '21 at 9:07
0
\$\begingroup\$

Class#newInstance is deprecated as it does not handle the checked exceptions of the default constructor call. So getting the default constructor (no arguments) and using Constructor#newInstance is the correct way.

Only problem: there might not exist a default constructor, only some constructor with parameters.

public static Object instantiate(String classFullName) throws ClassNotFoundException,
        NoSuchMethodException,
        InstantiationException,
        IllegalAccessException,
        IllegalArgumentException,
        InvocationTargetException {
    Class<?> type = Class.forName(classFullName);
    return type.getDeclaredConstructor().newInstance();
}

You might use Exception instead of the entire list.

\$\endgroup\$
5
  • 1
    \$\begingroup\$ "bypasses the default constructor" isn't exactly correct. It calls the default constructor, only it lets checked exceptions from that constructor escape the compiler checks. \$\endgroup\$ Nov 18 '21 at 12:06
  • \$\begingroup\$ @RalfKleberhoff of course you are right and I corrected the text. \$\endgroup\$
    – Joop Eggen
    Nov 18 '21 at 12:58
  • \$\begingroup\$ @Joop Eggan after reading your reply, I tried changing to a simple try catch and it works perfect, thanks. I never pass parameters to a constructor anyway, I only pass to methods needing the parameters. So if the class exist, there should be no errors. I tried to vote up on your reply, but I am no allowed yet. \$\endgroup\$ Nov 18 '21 at 18:40
  • \$\begingroup\$ @JimWhitaker try { ... } catch (NS;E | InE | IllAE | IllArE | ITE e) { throw new IllegalStateException(e); } is feasible too. \$\endgroup\$
    – Joop Eggen
    Nov 18 '21 at 20:22
  • \$\begingroup\$ A side question, does anyone know where the actual source code is that makes @Inject, jakarta.inject.Inject work, I have searched Github and Google, haven't fount it yet. \$\endgroup\$ Nov 19 '21 at 19:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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