JanusGraph / JanusGraph/janusgraph

Replace unnecessary Preconditions by assert

Open
#2,109 2 comments 2 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
5.8k
Forks
1.2k
Avg merge
13h 53m
Merged PRs (30d)
6

Description

In many places Preconditions are redundant and have no any sense. Many Preconditions are running in production and are checking something that has no any sense. I am suggestion to replace **unnecessary** `Preconditions` by `assert` because `assert` is disabled by default and is commonly used only for testing purposes or hints in code.
By using `assert` users will be able to `enable` those checks if they think they are needed, otherwise unnecessary checks will be disabled by default.

For example, place with the following check has no sense as we are throwing `NPE` when executing `additions.isEmpty()`, so `checkNotNull` is redundant.
```
Preconditions.checkNotNull(additions);
if (additions.isEmpty()){
additions=null;
}
```
Thus, we can simply replace it with the following code:
```
assert additions!=null;
if (additions.isEmpty()){
additions=null;
}
```

An example of needed checks might something like:
```
this.additions = Preconditions.checkNotNull(additions);
```

Contributor guide

Open the contributing guide

Research direction

No files, tests, or entry points are named. Start by inventorying the repository's Preconditions uses and distinguishing redundant checks from checks whose values are assigned or otherwise required. Done means the agreed unnecessary checks are replaced without changing required validation behavior, with relevant tests passing.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
databases
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.