4
\$\begingroup\$

I've been using my Dynamic Exception with C# for quite some time already and it saved me a lot of time. This means, I don't have to create a new exception class for each and every case. I wanted to have the same functionality on Android and in kotlin/java so I can do this:

fun main() {
    throw dynamicException("My", "Hallo exception!") // throws MyException
}

The DynamicException.kt file contains most of the code where the dynamicException function first initializes the source-code for the new exception by formatting a String then it uses the JavaCompiler to build the class and call the appropriate construtor. Either with or without the inner exception.

import java.io.File
import java.lang.reflect.Constructor
import java.net.URI
import java.net.URL
import java.net.URLClassLoader
import java.util.*
import javax.tools.DiagnosticCollector
import javax.tools.JavaFileObject
import javax.tools.SimpleJavaFileObject
import javax.tools.ToolProvider

fun dynamicException(name: String, message: String, inner: Throwable? = null): java.lang.Exception {
    val javaCompiler = ToolProvider.getSystemJavaCompiler()
    val diagnosticCollector = DiagnosticCollector<JavaFileObject>()

    val values = TreeMap<String, String>(String.CASE_INSENSITIVE_ORDER)
    values["name"] = name
    var sourceCode = SourceCodeJavaFileObject(
        "com.he-dev.${name}Exception",
        dynamicExceptionSourceCode.smartFormat(values)
    )
    javaCompiler.getTask(
        null,
        null,
        diagnosticCollector,
        null,
        null,
        arrayListOf(sourceCode)
    ).call()

    val classLoader = URLClassLoader.newInstance(arrayOf<URL>(File("").toURI().toURL()))

    var getCtor: () -> Constructor<out Any> = {
        val cls = Class.forName("${name}Exception", true, classLoader)
        val ctor = if (inner == null) {
            cls.getConstructor(String::class.java)
        } else {
            cls.getConstructor(String::class.java, Throwable::class.java)
        }
        ctor.makeAccessible()
    }

    return if (inner == null) {
        getCtor().newInstance(message) as java.lang.Exception
    } else {
        getCtor().newInstance(message, inner) as java.lang.Exception
    }
}

fun Constructor<out Any>.makeAccessible(): Constructor<out Any> {
    this.isAccessible = true
    return this
}


val dynamicExceptionSourceCode: String = """
public class {Name}Exception extends java.lang.Exception {
    public {Name}Exception(java.lang.String message) {
        super(message);
    }
    public {Name}Exception(java.lang.String message, java.lang.Throwable inner) {
        super(message, inner);
    }
}
""".trimIndent()

class SourceCodeJavaFileObject : SimpleJavaFileObject {
    private val sourceCode: CharSequence

    constructor(className: String, sourceCode: CharSequence) :
            super(
                URI.create("string:///" + className.replace('.', '/') + JavaFileObject.Kind.SOURCE.extension),
                JavaFileObject.Kind.SOURCE
            ) {
        this.sourceCode = sourceCode
    }

    override fun getCharContent(ignoreEncodingErrors: Boolean): CharSequence {
        return sourceCode
    }
}

The string formatting is done with a string extension that can replace patterns. I based it on my C# formatter. However, it's simpler because it doesn't not support value formatting.

import java.util.*

fun String.smartFormat(values: TreeMap<String, String>): String {
    val regex = Regex("""\{(?<name>[a-z][a-z0-9_.-]*)\}""", RegexOption.IGNORE_CASE)
    return regex.replace(this) {
        var key = it.groups["name"]?.value
        if (values.containsKey(key)) values[key]!! else it.value
    }
}

Is there anything that can be simplified or made even cleaner?


Disclaimer: Please let's not make it about whether this utility is a good or bad practice. I've used it many projects already and it stands the test of being super-helpful and super-efficient. I can discuss it on Software Engineering if you'd like to know more but here I'm only interested in improving the code.

Mod Note: The original asker added an extended explanation for their use of this pattern as a community wiki answer on the linked previous implementation of this code in C#.

\$\endgroup\$
  • 1
    \$\begingroup\$ @SimonForsberg true, it's easy but still, you have to write them. With this, you just throw exceptions... and catching them... mhmmm... I never knew why one would want to this ;-] It's for information purposes and for easier debugging, 99.99% there is nothing one can do about an exception but log it and break or repeat the operation so I see no value in catching anything but Exception. \$\endgroup\$ – t3chb0t Apr 22 at 12:46
  • 9
    \$\begingroup\$ That's exactly what the message is for. You don't need to have one kind of exception for every possible method name that can be missing, just have one generic class for MethodNotSupported and use an appropriate message on it for all the details that you need to indicate what went wrong and how to reproduce it. \$\endgroup\$ – Simon Forsberg Apr 22 at 16:21
  • 11
    \$\begingroup\$ If you don't want to catch specific exceptions, identified by their type, why would you want to introduce different exception types? Different messages will do the job of producing useful log entries. Your approach of invoking the compiler at runtime seems like a very complex solution to a problem that doesn't really exist. \$\endgroup\$ – Ralf Kleberhoff Apr 22 at 16:33
  • 7
    \$\begingroup\$ @t3chb0t If all you've got is the name of the exception, then you are doing something else wrong. If you want maximum usefulness of the name of your exception, why don't your name your exception BackgroundNotFound_C_Users_t3chb0t_Desktop_filename_png ? \$\endgroup\$ – Simon Forsberg Apr 22 at 20:02
  • 3
    \$\begingroup\$ I have downvoted this question because I don't consider it having a good enough motivation for why this code exist, which is an important part of posting a good question. The OP claims that they know the code is good, while my answer and the votes on my answer says otherwise. \$\endgroup\$ – Simon Forsberg Apr 23 at 8:32
5
\$\begingroup\$

Setting isAccessible to true

You seem to be always settings isAccessible to true. This is only needed if you are accessing methods outside their "access modifier", for example if you are trying to access a private method from another class.

Since you are only calling public methods (on public classes), this is not required.

Support for javax.tools is not for all android versions

You are using packages from javax.tools, this is not available on every android version, see the following SO question: NoClassDefFoundException when using javax.tools package, make sure to properly test on the oldest android version you are targetting.

To avoid these packages, manually define a class using byte arrays, and load that instead of the output of the compilation

\$\endgroup\$
  • \$\begingroup\$ This is the kind of answer I was counting on. Thanks! Sorry for not having documented it in my code, without modifying the isAccessible the constructor cannot be called. \$\endgroup\$ – t3chb0t Apr 23 at 3:31
17
\$\begingroup\$

Is there anything that can be simplified or made even cleaner?

Yes, don't invoke the Java compiler at runtime.

From your examples in a comment:

DynamicException.Create($"ExtractMethodName(memberName)}NotSupported", ...)

From an example on your earlier post (in C#)

throw ("SettingNotFoundException", $"Setting {fullName.ToString().QuoteWith("'")} not found.").ToDynamicException())

public BackgroundImageNotFoundException(string fileName) : base($"Where is the '{fileName}' image?") { }

Replace these with:

  • throw new MethodNotSupported(extractMethodName(memberName))
  • throw new UnsupportedOperationError(extractMethodName(memberName))
  • throw new IllegalStateException("Setting '" + fullName + "' not found")
  • throw new FileNotFoundException(fileName)

If you look at the subclasses of Java's Exception or RuntimeException (many of which also has a Kotlin version) you can probably find an already existing exception that does what you need, and you just need to add a message to it.

In a chat message related to your C# post you wrote:

In order to be able to track down a bug you need two pieces of information: The name of the exception and a message. With a generic exception I could just throw an Exception but the name of the exception should already be strong enough to tell what caused it, the message is just a hint.

You should already know what happend by not even reading the message.

I completely disagree with this. The message is not just a hint. To understand fully what happened and how to reproduce it you need to read the message.


As an extra bonus, here's how you define exceptions easily in Kotlin, and the approach I would recommend:

class MyException(message: String) : Exception(message)
class SomeOtherException(message: String) : Exception(message)
class UsefulException(message: String) : Exception(message)
class AnotherUsefulException(message: String) : Exception(message)

Please note that all this can be defined in the same file.

\$\endgroup\$
  • 9
    \$\begingroup\$ Disclaimer: This answer was written 10 seconds before the question was edited and a disclaimer was added. Either way, I still stand by this answer and I believe that your current approach is not good practice and adds unnecessary complexity. \$\endgroup\$ – Simon Forsberg Apr 22 at 16:45
  • \$\begingroup\$ Note that the original asker added an extended explanation for their use of this pattern as a community wiki answer on the linked previous implementation of this code in C#. \$\endgroup\$ – Vogel612 Apr 23 at 21:17

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.