This PR contains focused code quality improvements identified through systematic codebase analysis. All changes maintain backward compatibility and follow the project's contribution guidelines.#270
Open
surajsahani wants to merge 2 commits intoKotlin:developfrom
Conversation
…icit Unit returns - Remove redundant null-forgiving operator in DataType.of() - Add comprehensive KDoc to EngineFactory interface - Remove explicit Unit return types from ComplexArrays set() operators All changes maintain backward compatibility and follow Kotlin coding conventions.
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request focuses on code quality improvements across three files in the Multik library, including removing a redundant null-check operator, adding comprehensive KDoc documentation to the EngineFactory interface, and removing explicit Unit return types from operator functions. The changes align with Kotlin coding conventions and maintain backward compatibility.
Changes:
- Removed redundant
!!null-forgiving operator inDataType.of()after null check - Added KDoc documentation to
EngineFactoryinterface and itsgetEngine()method - Removed explicit
: Unitreturn type declarations fromComplexFloatArray.set()andComplexDoubleArray.set()operators
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| multik-core/src/commonMain/kotlin/org/jetbrains/kotlinx/multik/ndarray/data/DataType.kt | Removed redundant null-forgiving operator after null check |
| multik-core/src/commonMain/kotlin/org/jetbrains/kotlinx/multik/ndarray/complex/ComplexArrays.kt | Removed explicit Unit return types from set operators to align with Kotlin conventions |
| multik-core/src/commonMain/kotlin/org/jetbrains/kotlinx/multik/api/EngineFactory.kt | Added comprehensive KDoc documentation for the interface and getEngine method |
multik-core/src/commonMain/kotlin/org/jetbrains/kotlinx/multik/api/EngineFactory.kt
Outdated
Show resolved
Hide resolved
multik-core/src/commonMain/kotlin/org/jetbrains/kotlinx/multik/api/EngineFactory.kt
Outdated
Show resolved
Hide resolved
- Clarify that EngineFactory is a simple factory pattern - Move platform-specific discovery details to enginesProvider reference - Fix @throws tag to document IllegalStateException (from error()) instead of EngineMultikException - Add cross-reference to enginesProvider function
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes Made
1. Fixed Redundant Null Check in DataType.kt
File:
multik-core/src/commonMain/kotlin/org/jetbrains/kotlinx/multik/ndarray/data/DataType.ktIssue: The
of()function had a redundant null-forgiving operator (!!) after an elvis operator that already throws on null.Before:
After:
Impact: Improves code clarity and follows Kotlin best practices.
2. Added Comprehensive KDoc to EngineFactory Interface
File:
multik-core/src/commonMain/kotlin/org/jetbrains/kotlinx/multik/api/EngineFactory.ktIssue: Public API interface lacked documentation explaining its purpose, usage, and exception behavior.
Added:
Impact: Improves API discoverability and developer experience. Aligns with project documentation standards.
3. Removed Explicit Unit Return Types in ComplexArrays.kt
File:
multik-core/src/commonMain/kotlin/org/jetbrains/kotlinx/multik/ndarray/complex/ComplexArrays.ktIssue: Two
set()operator functions explicitly declared: Unitreturn type, which is redundant in Kotlin.Before:
After:
Impact: Follows Kotlin coding conventions and improves code consistency.
Testing
Backward Compatibility
✅ All changes maintain full backward compatibility:
: Unitis binary compatible)Checklist
developbranchAdditional Context
During the codebase analysis, several other improvement opportunities were identified for future PRs:
deepCopy()calls in arithmetic operationsThese are documented separately and can be addressed in follow-up contributions.