Skip to content

Commit

Permalink
Treat empty string in optionalInt or optionalFloat as null
Browse files Browse the repository at this point in the history
I did not apply this at the Marshaller level. This wouldn't apply to a db query
(where no row would be returned) or similar.

But in a querystring `series_id=1&season=' is better served by setting
get()->optionalInt('season') to null rather than throwing an exception.

It will certainly make returning eventreport.php to working order easier.

We *could* instead have the select set a magic constant for season=All of 'All'
or '99' or '-1' but that seems worse than this. (0 is already taken as it is
possible for a series to have a season 0 for some reason.)
  • Loading branch information
bakert committed Nov 10, 2024
1 parent 9125579 commit 4504473
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 2 deletions.
17 changes: 15 additions & 2 deletions gatherling/Helpers/Request.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public function int(string $key, int|false $default = false): int

public function optionalInt(string $key): ?int
{
return marshal($this->vars[$key] ?? null)->optionalInt();
return marshal($this->coalesceNumeric($key))->optionalInt();
}

public function string(string $key, string|false $default = false): string
Expand All @@ -39,7 +39,7 @@ public function float(string $key, float|false $default = false): float
/** @psalm-suppress PossiblyUnusedMethod */
public function optionalFloat(string $key): ?float
{
return marshal($this->vars[$key] ?? null)->optionalFloat();
return marshal($this->coalesceNumeric($key))->optionalFloat();
}

/** @return list<int> */
Expand Down Expand Up @@ -71,4 +71,17 @@ public function dictString(string $key): array
{
return marshal($this->vars[$key] ?? null)->dictString();
}

// Coalesce like ?? does, but additionally if the value is an empty string, return null.
// This is how we want to treat something like 'season=' in a querystring.
private function coalesceNumeric(string $key): mixed
{
if (!isset($this->vars[$key])) {
return null;
}
if ($this->vars[$key] === '') {
return null;
}
return $this->vars[$key];
}
}
6 changes: 6 additions & 0 deletions tests/Helpers/MarshallerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ public function testOptionalInt(): void
marshal(123.45)->optionalInt();
}

public function testOptionalIntThrowsOnEmptyString(): void
{
$this->expectException(MarshalException::class);
marshal('')->optionalInt();
}

public function testString(): void
{
$this->assertEquals('hello', marshal('hello')->string('key'));
Expand Down
13 changes: 13 additions & 0 deletions tests/Helpers/RequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ public function testIntMissing(): void
$request->int('baz');
}

public function testIntThrowsOnEmptyString(): void
{
$request = new Request(['foo' => '']);
$this->expectException(MarshalException::class);
$request->int('foo');
}

public function testOptionalInt(): void
{
$request = new Request(['foo' => '123', 'bar' => '123.4']);
Expand All @@ -34,6 +41,12 @@ public function testOptionalInt(): void
$request->optionalInt('bar');
}

public function testOptionalIntReturnsNullOnEmptyString(): void
{
$request = new Request(['foo' => '']);
$this->assertNull($request->optionalInt('foo'));
}

public function testFloat(): void
{
$request = new Request(['foo' => '123.45']);
Expand Down

0 comments on commit 4504473

Please sign in to comment.