While reading through the http.common package I noticed a handful of small, behavior-preserving
simplifications. None of them change observable behavior or the public API surface — they drop a throwaway
allocation, collapse two duplicated string builds, untangle a double-negative guard, name an anonymous
15-line block, and remove an always-true term from a predicate. Grouping them in one ticket since they all
live in the same subsystem and are individually tiny.
sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/Headers.kt:278-283 — addAll should iterate the backing map directly
Old code:
public fun addAll(headers: Headers): Builder =
apply {
headers.entries().forEach { (key, values) ->
headersMap.computeIfAbsent(key) { mutableListOf() }.addAll(values)
}
}
New code:
public fun addAll(headers: Headers): Builder =
apply {
headers.headersMap.forEach { (key, values) ->
headersMap.computeIfAbsent(key) { mutableListOf() }.addAll(values)
}
}
Usage — before → after:
val merged = Headers.Builder()
.add("Accept", "application/json")
.addAll(other)
.build()
val merged = Headers.Builder()
.add("Accept", "application/json")
.addAll(other)
.build()
call sites unchanged — behavior-preserving.
Why: entries() exists to hand callers a defensively-copied snapshot: it allocates a LinkedHashMap,
wraps every value list in Collections.unmodifiableList, and wraps the whole thing in
Collections.unmodifiableMap — all of which is discarded immediately when we only want to iterate. Reading
headers.headersMap directly (exactly as the copy constructor at line 133 already does) skips those
allocations while preserving insertion order and multi-value semantics, since the built map is a
LinkedHashMap and .addAll(values) only reads from each list.
API / Build: No public-API change — addAll's signature is untouched, so apiCheck is unaffected. The
nested Builder already reads the outer class's private headersMap in its copy constructor, so this
introduces no new visibility coupling.
sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/MediaType.kt:87-93 — reuse the fullType getter in toString
Old code:
override fun toString(): String {
val formattedParams =
parameters.entries.joinToString(separator = ";") { (key, value) ->
"$key=${formatParameterValue(value)}"
}
return if (formattedParams.isEmpty()) "$type/$subtype" else "$type/$subtype;$formattedParams"
}
New code:
override fun toString(): String {
val formattedParams =
parameters.entries.joinToString(separator = ";") { (key, value) ->
"$key=${formatParameterValue(value)}"
}
return if (formattedParams.isEmpty()) fullType else "$fullType;$formattedParams"
}
Usage — before → after:
MediaType.parse("text/plain;charset=utf-8").toString() // "text/plain;charset=utf-8"
MediaType.parse("text/plain;charset=utf-8").toString() // "text/plain;charset=utf-8"
call sites unchanged — behavior-preserving.
Why: The fullType getter already computes "$type/$subtype"; reusing it removes the two inline rebuilds
of that string and keeps the bare-type wire form defined in exactly one place.
API / Build: No public-API or behavioral change; apiCheck is unaffected.
sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/MediaType.kt:145-147, 182-184 — drop the double-negative in the wildcard guard
Old code:
of() (lines 145-147):
require(!(type == "*" && subtype != "*")) {
"Invalid media type format: type=$type, subtype=$subtype"
}
parse() (lines 182-184):
require(!(type == "*" && subtype != "*")) {
"Invalid media type format: $mediaType"
}
New code:
of():
require(type != "*" || subtype == "*") {
"Invalid media type format: type=$type, subtype=$subtype"
}
parse():
require(type != "*" || subtype == "*") {
"Invalid media type format: $mediaType"
}
Usage — before → after:
MediaType.of("*", "*") // ok
MediaType.of("*", "json") // throws IllegalArgumentException — wildcard type requires wildcard subtype
MediaType.of("*", "*") // ok
MediaType.of("*", "json") // throws IllegalArgumentException — wildcard type requires wildcard subtype
call sites unchanged — behavior-preserving.
Why: By De Morgan's law !(type == "*" && subtype != "*") is type != "*" || subtype == "*", which reads
straight off as the documented invariant — "a wildcard type is only allowed when the subtype is also a
wildcard." Same truth table, no double negative to decode, and the error messages are unchanged.
API / Build: No public-API or behavioral change; apiCheck is unaffected.
sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/MediaType.kt:204-221 — extract the quoted-value unescape into a named function
Old code:
val value =
if (rawValue.startsWith("\"") && rawValue.endsWith("\"") && rawValue.length >= 2) {
val inner = rawValue.substring(1, rawValue.length - 1)
val sb = StringBuilder(inner.length)
var i = 0
while (i < inner.length) {
if (inner[i] == '\\' && i + 1 < inner.length) {
sb.append(inner[i + 1])
i += 2
} else {
sb.append(inner[i])
i++
}
}
sb.toString()
} else {
rawValue
}
New code:
The inline block becomes a single call, and the unescape logic moves to a companion-level helper that mirrors
the existing formatParameterValue:
val value = unescapeQuotedValue(rawValue)
/**
* Strips surrounding double-quotes from a parameter [rawValue] and unescapes its
* quoted-pair sequences (`\"` -> `"`, `\\` -> `\`), per RFC 7230 §3.2.6. A value that
* is not a `quoted-string` is returned unchanged. Inverse of [formatParameterValue].
*/
private fun unescapeQuotedValue(rawValue: String): String {
if (!(rawValue.startsWith("\"") && rawValue.endsWith("\"") && rawValue.length >= 2)) {
return rawValue
}
val inner = rawValue.substring(1, rawValue.length - 1)
val sb = StringBuilder(inner.length)
var i = 0
while (i < inner.length) {
if (inner[i] == '\\' && i + 1 < inner.length) {
sb.append(inner[i + 1])
i += 2
} else {
sb.append(inner[i])
i++
}
}
return sb.toString()
}
Usage — before → after:
val value =
if (rawValue.startsWith("\"") && rawValue.endsWith("\"") && rawValue.length >= 2) {
// ... 15 lines of inline quoted-pair unescape ...
} else {
rawValue
}
val value = unescapeQuotedValue(rawValue)
External MediaType.parse(...) results are unchanged — the extraction is internal to the parse loop.
Why: This 15-line block is the inverse of formatParameterValue but sits anonymously inside an
associate { } lambda, so the parameter-parsing loop is hard to skim. Lifting it to a named
private fun unescapeQuotedValue (next to its mirror image) names the operation and flattens the parse
body.
API / Build: The new symbol is private, so there is no public-API surface and apiCheck is unaffected.
sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/ETag.kt:92-94 — drop the redundant !startsWith("W/") term
Old code:
val isStrongForm =
trimmed.length >= 2 &&
!trimmed.startsWith("W/") && trimmed.startsWith("\"") && trimmed.endsWith("\"")
New code:
val isStrongForm =
trimmed.length >= 2 &&
trimmed.startsWith("\"") && trimmed.endsWith("\"")
Usage — before → after:
ETag.parse("\"abc\"") // strong ETag
ETag.parse("W/\"abc\"") // weak ETag
ETag.parse("W/abc") // throws IllegalArgumentException — malformed
ETag.parse("\"abc\"") // strong ETag
ETag.parse("W/\"abc\"") // weak ETag
ETag.parse("W/abc") // throws IllegalArgumentException — malformed
call sites unchanged — behavior-preserving.
Why: startsWith("\"") already requires the first character to be ", which a string starting with W/
can never be, so !startsWith("W/") is always true whenever the strong-form test reaches it. Dropping the
redundant term leaves the same result set and a clearer predicate.
API / Build: No public-API or behavioral change; apiCheck is unaffected.
While reading through the
http.commonpackage I noticed a handful of small, behavior-preservingsimplifications. None of them change observable behavior or the public API surface — they drop a throwaway
allocation, collapse two duplicated string builds, untangle a double-negative guard, name an anonymous
15-line block, and remove an always-true term from a predicate. Grouping them in one ticket since they all
live in the same subsystem and are individually tiny.
sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/Headers.kt:278-283 —
addAllshould iterate the backing map directlyOld code:
New code:
Usage — before → after:
call sites unchanged — behavior-preserving.
Why:
entries()exists to hand callers a defensively-copied snapshot: it allocates aLinkedHashMap,wraps every value list in
Collections.unmodifiableList, and wraps the whole thing inCollections.unmodifiableMap— all of which is discarded immediately when we only want to iterate. Readingheaders.headersMapdirectly (exactly as the copy constructor at line 133 already does) skips thoseallocations while preserving insertion order and multi-value semantics, since the built map is a
LinkedHashMapand.addAll(values)only reads from each list.API / Build: No public-API change —
addAll's signature is untouched, soapiCheckis unaffected. Thenested
Builderalready reads the outer class's privateheadersMapin its copy constructor, so thisintroduces no new visibility coupling.
sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/MediaType.kt:87-93 — reuse the
fullTypegetter intoStringOld code:
New code:
Usage — before → after:
call sites unchanged — behavior-preserving.
Why: The
fullTypegetter already computes"$type/$subtype"; reusing it removes the two inline rebuildsof that string and keeps the bare-type wire form defined in exactly one place.
API / Build: No public-API or behavioral change;
apiCheckis unaffected.sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/MediaType.kt:145-147, 182-184 — drop the double-negative in the wildcard guard
Old code:
of()(lines 145-147):parse()(lines 182-184):New code:
of():parse():Usage — before → after:
call sites unchanged — behavior-preserving.
Why: By De Morgan's law
!(type == "*" && subtype != "*")istype != "*" || subtype == "*", which readsstraight off as the documented invariant — "a wildcard type is only allowed when the subtype is also a
wildcard." Same truth table, no double negative to decode, and the error messages are unchanged.
API / Build: No public-API or behavioral change;
apiCheckis unaffected.sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/MediaType.kt:204-221 — extract the quoted-value unescape into a named function
Old code:
New code:
The inline block becomes a single call, and the unescape logic moves to a companion-level helper that mirrors
the existing
formatParameterValue:Usage — before → after:
External
MediaType.parse(...)results are unchanged — the extraction is internal to the parse loop.Why: This 15-line block is the inverse of
formatParameterValuebut sits anonymously inside anassociate { }lambda, so the parameter-parsing loop is hard to skim. Lifting it to a namedprivate fun unescapeQuotedValue(next to its mirror image) names the operation and flattens theparsebody.
API / Build: The new symbol is
private, so there is no public-API surface andapiCheckis unaffected.sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/ETag.kt:92-94 — drop the redundant
!startsWith("W/")termOld code:
New code:
Usage — before → after:
call sites unchanged — behavior-preserving.
Why:
startsWith("\"")already requires the first character to be", which a string starting withW/can never be, so
!startsWith("W/")is always true whenever the strong-form test reaches it. Dropping theredundant term leaves the same result set and a clearer predicate.
API / Build: No public-API or behavioral change;
apiCheckis unaffected.