Skip to content

Commit d694874

Browse files
committed
refactor: drop comments that narrate instead of constrain
Keep a comment only when it says something the code cannot: a non-obvious why, an external constraint, or an invariant a future edit would break. Comments that narrate a test, restate an assertion, or argue the code is correct are deleted; the tests are the explanation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HMKbYgbsoFdKiiaxXB6jh2
1 parent d75e355 commit d694874

9 files changed

Lines changed: 4 additions & 44 deletions

‎src/Http/SerializingClient.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,6 @@ private static function serialize_options(array $options): array
102102
);
103103

104104
if ($serialized === "") {
105-
// Nothing serialized must mean no query at all, not a bare
106-
// trailing `?`.
107105
unset($options["query"]);
108106
} else {
109107
$options["query"] = $serialized;

‎src/NullValue.php‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,6 @@ enum NullValue
3232
{
3333
/**
3434
* Sentinel for a param explicitly set to null.
35-
*
36-
* An enum case cannot be instantiated or subclassed, so this is the only
37-
* value of this type, and `$value instanceof NullValue` detects it.
3835
*/
3936
case NULL;
4037

‎src/StrictUrlSearchParamsSerializer.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,6 @@ public static function update(
4545
UrlSearchParamsSerializer::update($search_params, $params);
4646

4747
if (count($search_params) > 0) {
48-
// Replaced rather than repeated, and appended after the sort so
49-
// it always sits last.
5048
$search_params->delete("_strict");
5149
$search_params->append("_strict", "true");
5250
}

‎src/UnserializableParamError.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,8 @@
33
namespace Seam;
44

55
/**
6-
* Error thrown when a request param could not be serialized.
7-
*
8-
* It is thrown before any request is sent, so it never means the API
9-
* rejected the param, only that this SDK could not represent it.
6+
* Error thrown when a request param could not be serialized, before any
7+
* request is sent.
108
*/
119
class UnserializableParamError extends \InvalidArgumentException
1210
{

‎src/UrlSearchParamsSerializer.php‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -286,9 +286,8 @@ private static function format_number(string $name, float $value): string
286286
private static function shortest_digits(float $value): array
287287
{
288288
// At -1, PHP's float-to-string conversion produces the shortest
289-
// string that round-trips, which is exactly the digit string the
290-
// algorithm needs. The ini setting is restored because it also
291-
// affects the caller's own serialize() and json_encode() calls.
289+
// string that round-trips. Restored because the setting also affects
290+
// the caller's own serialize() and json_encode() calls.
292291
$precision = ini_set("serialize_precision", "-1");
293292

294293
try {
@@ -306,8 +305,6 @@ private static function shortest_digits(float $value): array
306305
$matches,
307306
) !== 1
308307
) {
309-
// Unreachable for a finite positive float, but a silent
310-
// mis-parse would serialize a wrong number.
311308
throw new \RuntimeException(
312309
"Could not parse the PHP float representation: {$repr}",
313310
);

‎tests/NullValueTest.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ public function testReplaceCopiesStdClassObjectsWithoutMutatingThem(): void
6767
$this->assertNotSame($payload, $replaced);
6868
$this->assertNull($replaced->name);
6969
$this->assertNull($replaced->nested->code);
70-
// The caller's payload is untouched.
7170
$this->assertSame(NullValue::NULL, $payload->name);
7271
$this->assertSame(NullValue::NULL, $payload->nested->code);
7372
}

‎tests/SearchParamsTest.php‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,6 @@
1010
use Seam\UnserializableParamError;
1111
use Tests\Support\RecordingClient;
1212

13-
/**
14-
* Asserts on the raw query string the client puts on the wire, not on a
15-
* re-parsed version of it, which would hide encoding differences.
16-
*/
1713
final class SearchParamsTest extends TestCase
1814
{
1915
private const DEVICES = ["devices" => []];
@@ -58,12 +54,6 @@ public function testClientSerializesSearchParams(): void
5854
);
5955
}
6056

61-
/**
62-
* The serializer and Guzzle disagree on exactly two characters: Guzzle
63-
* escapes `*` and leaves `~` alone, so a query Guzzle encodes is not the
64-
* one the serializer produced. Handing Guzzle the raw string keeps ours,
65-
* including through resolution against the client's base URL.
66-
*/
6757
public function testClientDoesNotReencodeTheSerializedSearchParams(): void
6858
{
6959
$recording = RecordingClient::repeating(
@@ -90,8 +80,6 @@ public function testClientSerializesEmptyArraysToAnEmptyValue(): void
9080
"query" => ["device_ids" => []],
9181
]);
9282

93-
// Not omitted and not a bare name: the parser reads `device_ids=`
94-
// as the empty array, while no param at all means unfiltered.
9583
$this->assertSame(
9684
"device_ids=&_strict=true",
9785
$recording->request()->getUri()->getQuery(),
@@ -175,10 +163,6 @@ public function testClientSerializesSearchParamsOfEveryVerb(): void
175163
}
176164
}
177165

178-
/**
179-
* A query already given as a string is a representation the caller
180-
* chose, so it is forwarded to Guzzle untouched.
181-
*/
182166
public function testClientPassesSearchParamsItDidNotSerializeToGuzzle(): void
183167
{
184168
$recording = RecordingClient::repeating(
@@ -211,7 +195,6 @@ public function testClientRejectsASearchParamItCannotSerialize(): void
211195
$this->assertSame("search", $error->getName());
212196
}
213197

214-
// The error is raised before any request goes out.
215198
$this->assertSame(0, $recording->request_count());
216199
}
217200

‎tests/StrictUrlSearchParamsSerializerTest.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@ public function testAddsStrictToNonEmptyQueryStrings(): void
2121

2222
public function testAppendsStrictAfterTheSortedParams(): void
2323
{
24-
// `_strict` would sort between the uppercase and lowercase names;
25-
// it is appended after the sort so it always sits last.
2624
$this->assertSame(
2725
"B=2&a=1&_strict=true",
2826
StrictUrlSearchParamsSerializer::serialize(["a" => 1, "B" => 2]),

‎tests/UrlSearchParamsSerializerTest.php‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ public function testSerializesString(): void
3636

3737
public function testRemovesTheEmptyString(): void
3838
{
39-
// Serializing the empty string would conflict with NullValue::NULL.
4039
$this->assertSame("", self::serialize(["foo" => ""]));
4140
$this->assertSame(
4241
"foo=d",
@@ -75,8 +74,6 @@ public function testSerializesFloat(): void
7574

7675
public function testSerializesFloatUsingTheEcmascriptNumberFormat(): void
7776
{
78-
// A float is serialized exactly as JavaScript would serialize the
79-
// number, which is not always the same as the PHP string cast.
8077
$this->assertSame("foo=1", self::serialize(["foo" => 1.0]));
8178
$this->assertSame("foo=0", self::serialize(["foo" => -0.0]));
8279
$this->assertSame("foo=100", self::serialize(["foo" => 100.0]));
@@ -200,7 +197,6 @@ public function testSerializesMutableDatetime(): void
200197
"now=2025-02-24T18%3A44%3A39.000Z",
201198
self::serialize(["now" => $now]),
202199
);
203-
// Converting to UTC must not mutate the caller's value.
204200
$this->assertSame("2025-02-24T18:44:39+00:00", $now->format("c"));
205201
}
206202

@@ -342,8 +338,6 @@ public function testSortsParamsByName(): void
342338

343339
public function testSortsParamsByUtf16CodeUnit(): void
344340
{
345-
// UTF-8 byte order and code-point order would both put U+FFFF first;
346-
// only UTF-16 code-unit order puts the astral emoji first.
347341
$this->assertSame(
348342
"%F0%9F%98%80=2&%EF%BF%BF=1",
349343
self::serialize(["\u{FFFF}" => 1, "\u{1F600}" => 2]),
@@ -407,8 +401,6 @@ public function testCannotSerializeNestedKeysContainingADot(): void
407401

408402
public function testCannotSerializeNonStringKeys(): void
409403
{
410-
// PHP casts a numeric-string key to an integer, so both spellings
411-
// arrive here as the same unserializable key.
412404
$this->expectException(UnserializableParamError::class);
413405

414406
self::serialize(["foo" => [1 => "a", "b" => "c"]]);

0 commit comments

Comments
 (0)