Skip to content

Conversation

@nastra
Copy link
Contributor

@nastra nastra commented Dec 8, 2021

Motivation

This is relying on https://immutables.github.io/ (Apache License 2.0), which allows generating immutable objects and builders via annotation processing.

  • Immutable objects are serialization ready (including JSON and its binary forms)
  • Supports lazy, derived and optional attributes
  • Immutable objects are constructed once, in a consistent state, and can be safely shared
    • Will fail if mandatory attributes are missing
    • Cannot be sneakily modified when passed to other code
  • Immutable objects are naturally thread-safe and can therefore be safely shared among threads
    • No excessive copying
    • No excessive synchronization
  • Object definitions are pleasant to write and read
    • No boilerplate setter and getters
    • No ugly IDE-generated hashCode, equals and toString methods that end up being stored in source control.

Assertions.assertThatThrownBy(() -> TableIdentifier.of(Namespace.empty(), null))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Invalid table name: null or empty");
.isInstanceOf(NullPointerException.class)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is because the Immutables lib does an Objects.requireNonNull(name, "name") for everything that isn't marked as @Nullable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alternatively, we could add @Value.Style(throwForNullPointer = IllegalArgumentException.class) to throw an IAE for nulls

@nastra nastra force-pushed the use-immutables branch 2 times, most recently from 68fde04 to 011d4e5 Compare December 9, 2021 07:34
@nastra nastra requested review from rdblue and rymurr February 2, 2022 10:47
Copy link
Contributor

@rymurr rymurr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

safer, less code and no change in behaviour! Im in!

@rdblue any comments?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants