I used the AI Self-Review feature, and it gave me ...

# getting-started

[Daniel Pitts]

I used the AI Self-Review feature, and it gave me this:

The inner `None` object, used in the `Maybe` class to represent an absent value, is defined using `data object`. Although this is suitable for describing uniqueness and immutability, `data object` is over-specified here, as it incurs unnecessary memory allocation inefficiency compared to plain `object`. Consider replacing `data object None` with `object None`.

This seems like its wrong...

[Seri]

Give this a read if you want to learn more about `data object`s and why they were introduced to the language

https://github.com/Kotlin/KEEP/blob/main/proposals/KEEP-0317-data-objects.md

[Daniel Pitts]

I'm not seeing where the memory allocation comes in. Are data object allocated differently than non-data objects?

[Youssef Shoaib [MOD]]

Yeah the AI is just hallucinating completely. The only difference between object and data object is toString and equals, that's it!

[Daniel Pitts]

That's what I thought 😉. Thanks for confirming.

[CLOVIS]

@Youssef Shoaib [MOD] what's the difference in equals ?

[Daniel Pitts]

According to the KEEP, equals handles cases where more than one instance might have been created. How it does that, I'm not sure.

[CLOVIS]

interesting, I never knew…

[CLOVIS]

thanks!

[Youssef Shoaib [MOD]]

It uses an instance check other is Foo instead of the default JVM behaviour of reference equality other === this. The idea being that, if a serialization framework accidentally creates multiple copies of a singleton object, they'll at least compare equal to each other.

[Daniel Pitts]

is it return other is This or return other === this || other is This? Functionally identical, just wondering about runtime cost (if any)

[Youssef Shoaib [MOD]]

That I'm not sure about. It could be either way honestly. The latter is what people on the JVM normally do, the former would make more sense though.

[CLOVIS]

The JVM people themselves recommend the former though.

https://www.youtube.com/watch?v=kuzjX_efuDs▾

[Youssef Shoaib [MOD]]

Btw, turns out they also override hashCode so that it's the same for all instances of the object type. Makes sense since that's required for equals to function properly

[Daniel Pitts]

Another hallucination by AI (self-review):

In the refactored implementation of `toTree`, `DeepRecursiveFunction` is used to convert mutable nodes to tree nodes. While this approach is modern and succinct, recursive functions may hit `StackOverflowError` for deeply nested structures unless Kotlin ensures proper tail-recursion optimizations. Consider testing or adding safeguards for handling large, deeply-nested datasets.

Is there a place to report these to help improve the feature?

[Youssef Shoaib [MOD]]

It's just hallucinations in the underlying model. You can try tweak the prompt I guess, or change to a better model.

[Daniel Pitts]

There isn't a prompt I enter, its the "AI Self-Review" tool from JetBrains. Not sure I can change the model, but I'll check settings.

[Arjan van Wieringen]

This is inherent to LLMs. Its largest flaw.

[Daniel Pitts]

I’m aware of the flaws, but they can be mitigated a little or at least improved. The only way that will happen is if the dev is aware of the problem, hence wanting a way to report the hallucinations.

[Arjan van Wieringen]

I totally agree. Now it is a bad feature that is not thought out well and presents wrong ideas as facts. Exactly the problem when using LLMs wrong

[Daniel Pitts]

At least the commit message generation is better than Copilots lol.