-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Small performance changes #18
base: trunk
Are you sure you want to change the base?
Conversation
3 x faster and fix phpstan error
This PR have several changes too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the notes I've added are about only returning simple values/variables. This makes it easier to work on later without having to break out return expressions.
@@ -30,6 +30,7 @@ public function __construct( string $db ) { | |||
$sql = 'SELECT name FROM sqlite_master WHERE type="table" AND name="storage"'; | |||
verbose( "SQLite: $sql" ); | |||
$table_check = self::$db->querySingle( $sql ); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some extra tabs on this line.
} else { | ||
return true; | ||
} | ||
return (bool) $query->execute(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can appreciate removing the if/else combination. However, return should be a simple variable/value instead of an expression. This makes it easier to work with the results of $query->execute()
later on, if needed ( like debugging an unexpected behavior ).
|
||
return false; | ||
return (bool) $query->execute(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same item here about returning simple values/variables.
return false; | ||
} | ||
|
||
$results = $this->set( $key, $flags, $exptime, "{$results[0]['value']}$value" ); | ||
return $results; | ||
return $this->set( $key, $flags, $exptime, "{$results[0]['value']}$value" ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another item here of not returning a simple value/variable.
|
||
return false; | ||
return ( $query->execute() !== false && self::$db->changes() === 1 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only return a simple value/variable.
return false; | ||
} | ||
|
||
$results = $this->set( $key, $flags, $exptime, "$value{$results[0]['value']}" ); | ||
return $results; | ||
return $this->set( $key, $flags, $exptime, "$value{$results[0]['value']}" ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only return simple values/variables.
return false; | ||
} | ||
|
||
$result = $this->set( $key, $flags, $exptime, $value ); | ||
return $result; | ||
return $this->set( $key, $flags, $exptime, $value ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only return simple values/variables.
|
||
return false; | ||
return (bool) $query->execute(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only return simple value/variables.
@@ -255,29 +220,23 @@ public function stat_curr_items() { | |||
$result = $query->execute(); | |||
|
|||
if ( $result !== false ) { | |||
$row = $result->fetchArray( SQLITE3_ASSOC ); | |||
return $row['curr_items']; | |||
return $result->fetchArray( SQLITE3_ASSOC ) ['curr_items']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only return simple values/variables.
return false; | ||
} | ||
|
||
$result = $this->set( | ||
return $this->set( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only return simple values/variables.
$this->set()
always return bool[] === $results
is 3 x faster thancount( $results ) === 0
, and fix phpstan errorThe methods that return bool,
(bool) SQLite3Result|false
Used
(bool) $result
because is more semantic than($result)
.Changed type hint return in
get()
, always is array.