Looking at some kotlin code and found a data class...

# getting-started

[Colton Idle]

Looking at some kotlin code and found a data class being declared, and inside of it there's a companion object that builds the data class. That seems smelly right. Does it actually break any "rule"... like "data classes are data classes, and not [blank]" is what i want to say. lol

[Rob Elliot]

I don't think I understand why you think that's smelly?

[joseph_ivie]

I think this is pretty normal if there's some kind of special way one might construct an instance with restrictions. However, there'd better be a good reason because the default constructor is very nice for most cases.

[Colton Idle]

Because a data class is a value class? so it seems like putting any methods inside of it (a builder + another random method) just seems off.

[Rob Elliot]

I don't follow that at all. For a start a companion object is often just a way to group and namespace functions.

[Rob Elliot]

And why shouldn't you have methods on a data class that represent calculations from its state?

[joseph_ivie]

I think I'd need to see the specific case to really judge. I can see some good reasons to do all of those things. However, there's also a good chance that the code in question is just plain weird.

[Chris Lee]

i’ll occasionally have factory functions in data class companion objects - and mark the constructor as private - to encapsulate construction, where construction is beyond “pass parameters to constructor”.

[Riccardo Lippolis]

FYI: when making the data class constructor private, consider also using the @ConsistentCopyVisibility annotation on the data class, or setting the -Xconsistent-data-class-copy-visibility compiler flag. This will make sure the copy function of the data class also has the private visibility. Otherwise, it would still be possible to create 'invalid' data class instances using that copy function, which is public by default.

(in future versions (2.3?) the copy function will automatically get the same visibility as the primary constructor, see: https://youtrack.jetbrains.com/issue/KT-11914/Confusing-data-class-copy-with-private-constructor)

[Chris Lee]

💯 i consistently set “-Xconsistent-data-class-copy-visibility” to force the new behaviour.

[Klitos Kyriacou]

> Because a data class is a value class? so it seems like putting any methods inside of it (a builder + another random method) just seems off.
I would call it a "value-based" class, as "value class" means something else. Data classes are useful for implementing this sort of value-based classes, but it doesn't mean they should only be a simple collection of fields. It's common for them to have methods for things like transformations. It would be a bad smell if they had classes that did things not related directly to their content.

Also, there's no issue with having a companion object inside a data class. A companion object is a separate class, it just happens to be possible to use the same name as the class it's in.

[Edgar Avuzi]

I prefer using MapStruct mappers (or custom-written mappers) as injectable components to transform data into your data classes.

Companion object functions are essentially instance methods on a static object (a singleton). Placing large amounts of business logic inside static functions reduces maintainability - a common disease in many Kotlin projects. As the project grows, this often leads to major refactoring work to move logic from static functions into proper components, when that logic needs dependencies from the DI container
https://medium.com/@avuzia/kotlin-spring-the-hidden-cost-of-static-methods-35e134c07602

[Edgar Avuzi]

Limited Dependency Injection: Static methods cannot take advantage of Spring’s dependency injection. If a method suddenly needs a parameter from the Spring context, developers must either refactor the static method or introduce workarounds that make the code more complex.

The post is for Spring, but I think it's applicable to any code that relies on DI

[Edgar Avuzi]

All of the Kotlin projects that I joined had that issue, costly refactorings that take weeks to accomplish due to ppl place business logic in static functions (or instance methods of static singleton). And yet a few ppl understand the root cause.

If anyone wants to kill maintainability of a project with DI, they should place business logic into static functions, constructors and inheritance trees as much as possible - the same issue - the distance from DI aware code to deeply nested static functions is big - end up passing all the stuff manually through components (functions) - increases code rigidity and coupling