Skip to main content
Code Review

Return to Answer

added 215 characters in body
Source Link
Quentin Pradet
  • 7.1k
  • 1
  • 25
  • 44

reader from the future! PHP 7.0 introduced ?? which is a null coalescing operator, just use that: https://www.php.net/manual/en/migration70.new-features.php#migration70.new-features.null-coalesce-op .


Before dealing with your question, I'd like to point out that you probably want to use empty() instead of comparing with an empty string and that if you really want to compare to an empty string, use the === operator.

Anyway, default initialization is a real problem in many languages. C# even has a ?? operator to solve this kind of issue! So, let's try to make it better to make your code better step by step.

Ternary operator no more

The one-liner is too long, and using the ternary operator on multiple lines is a bad idea: that's what if-statement are for:

if (empty($newsItems[0]['image_url'])) {
 $newsItems[0]['image_url'] = '/img/cat_placeholder.jpg';
}

This is more readable for two reasons : it's easier to understand the syntax, but more importantly, it's easier to see that you simply want a default value when there's nothing. We don't need the 'else' part which is confusing with the ternary operator.

Note: as mentioned by Simon Scarfe and corrected by mseancole, PHP also has some special ternary syntax to do this:

$newsItems[0]['image_url'] = $newsItems[0]['image_url'] ?: 'img/cat_placeholder.jpg';

Factorize it!

If you're doing this only once or twice, then all is good, but otherwise you'd want to factorize it into a function, since you don't repeat yourself, right? The simple way is:

function default_value($var, $default) {
 return empty($var) ? $default : $var;
}
$newsItems[0]['image_url'] = default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

(The ternary operator does make sense here since variable names are short and both branches of the condition are useful.)

However, we're looking up $newsItems[0]['image_url'] when calling default_value, and this is possibly not defined, and will raise an error/warning. If that's a concern (it should), stick to the first version, or look at this other answer that gives a more robust solution at the expense of storing PHP code as a a string and thus cannot be checked syntactically.

Still too long

If we don't care about the warning/error, can we do better? Yes we can! We're writing the variable name twice, but passing by reference can help us here:

function default_value(&$var, $default) {
 if (empty($var)) {
 $var = $default;
 }
}
default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

It's much shorter, and feels more declarative: you can look at a bunch of default_value calls and see what the default values are instantly. But we still have the same warning/error issue.

Before dealing with your question, I'd like to point out that you probably want to use empty() instead of comparing with an empty string and that if you really want to compare to an empty string, use the === operator.

Anyway, default initialization is a real problem in many languages. C# even has a ?? operator to solve this kind of issue! So, let's try to make it better to make your code better step by step.

Ternary operator no more

The one-liner is too long, and using the ternary operator on multiple lines is a bad idea: that's what if-statement are for:

if (empty($newsItems[0]['image_url'])) {
 $newsItems[0]['image_url'] = '/img/cat_placeholder.jpg';
}

This is more readable for two reasons : it's easier to understand the syntax, but more importantly, it's easier to see that you simply want a default value when there's nothing. We don't need the 'else' part which is confusing with the ternary operator.

Note: as mentioned by Simon Scarfe and corrected by mseancole, PHP also has some special ternary syntax to do this:

$newsItems[0]['image_url'] = $newsItems[0]['image_url'] ?: 'img/cat_placeholder.jpg';

Factorize it!

If you're doing this only once or twice, then all is good, but otherwise you'd want to factorize it into a function, since you don't repeat yourself, right? The simple way is:

function default_value($var, $default) {
 return empty($var) ? $default : $var;
}
$newsItems[0]['image_url'] = default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

(The ternary operator does make sense here since variable names are short and both branches of the condition are useful.)

However, we're looking up $newsItems[0]['image_url'] when calling default_value, and this is possibly not defined, and will raise an error/warning. If that's a concern (it should), stick to the first version, or look at this other answer that gives a more robust solution at the expense of storing PHP code as a a string and thus cannot be checked syntactically.

Still too long

If we don't care about the warning/error, can we do better? Yes we can! We're writing the variable name twice, but passing by reference can help us here:

function default_value(&$var, $default) {
 if (empty($var)) {
 $var = $default;
 }
}
default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

It's much shorter, and feels more declarative: you can look at a bunch of default_value calls and see what the default values are instantly. But we still have the same warning/error issue.

reader from the future! PHP 7.0 introduced ?? which is a null coalescing operator, just use that: https://www.php.net/manual/en/migration70.new-features.php#migration70.new-features.null-coalesce-op .


Before dealing with your question, I'd like to point out that you probably want to use empty() instead of comparing with an empty string and that if you really want to compare to an empty string, use the === operator.

Anyway, default initialization is a real problem in many languages. C# even has a ?? operator to solve this kind of issue! So, let's try to make it better to make your code better step by step.

Ternary operator no more

The one-liner is too long, and using the ternary operator on multiple lines is a bad idea: that's what if-statement are for:

if (empty($newsItems[0]['image_url'])) {
 $newsItems[0]['image_url'] = '/img/cat_placeholder.jpg';
}

This is more readable for two reasons : it's easier to understand the syntax, but more importantly, it's easier to see that you simply want a default value when there's nothing. We don't need the 'else' part which is confusing with the ternary operator.

Note: as mentioned by Simon Scarfe and corrected by mseancole, PHP also has some special ternary syntax to do this:

$newsItems[0]['image_url'] = $newsItems[0]['image_url'] ?: 'img/cat_placeholder.jpg';

Factorize it!

If you're doing this only once or twice, then all is good, but otherwise you'd want to factorize it into a function, since you don't repeat yourself, right? The simple way is:

function default_value($var, $default) {
 return empty($var) ? $default : $var;
}
$newsItems[0]['image_url'] = default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

(The ternary operator does make sense here since variable names are short and both branches of the condition are useful.)

However, we're looking up $newsItems[0]['image_url'] when calling default_value, and this is possibly not defined, and will raise an error/warning. If that's a concern (it should), stick to the first version, or look at this other answer that gives a more robust solution at the expense of storing PHP code as a a string and thus cannot be checked syntactically.

Still too long

If we don't care about the warning/error, can we do better? Yes we can! We're writing the variable name twice, but passing by reference can help us here:

function default_value(&$var, $default) {
 if (empty($var)) {
 $var = $default;
 }
}
default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

It's much shorter, and feels more declarative: you can look at a bunch of default_value calls and see what the default values are instantly. But we still have the same warning/error issue.

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

Before dealing with your question, I'd like to point out that you probably want to use empty() instead of comparing with an empty string and that if you really want to compare to an empty string, use the === operator.

Anyway, default initialization is a real problem in many languages. C# even has a ?? operator to solve this kind of issue! So, let's try to make it better to make your code better step by step.

Ternary operator no more

The one-liner is too long, and using the ternary operator on multiple lines is a bad idea: that's what if-statement are for:

if (empty($newsItems[0]['image_url'])) {
 $newsItems[0]['image_url'] = '/img/cat_placeholder.jpg';
}

This is more readable for two reasons : it's easier to understand the syntax, but more importantly, it's easier to see that you simply want a default value when there's nothing. We don't need the 'else' part which is confusing with the ternary operator.

Note: as mentioned by Simon Scarfe and corrected by mseancole, PHP also has some special ternary syntax to do this:

$newsItems[0]['image_url'] = $newsItems[0]['image_url'] ?: 'img/cat_placeholder.jpg';

Factorize it!

If you're doing this only once or twice, then all is good, but otherwise you'd want to factorize it into a function, since you don't repeat yourself, right? The simple way is:

function default_value($var, $default) {
 return empty($var) ? $default : $var;
}
$newsItems[0]['image_url'] = default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

(The ternary operator does make sense here since variable names are short and both branches of the condition are useful.)

However, we're looking up $newsItems[0]['image_url'] when calling default_value, and this is possibly not defined, and will raise an error/warning. If that's a concern (it should), stick to the first version, or look at this other answer this other answer that gives a more robust solution at the expense of storing PHP code as a a string and thus cannot be checked syntactically.

Still too long

If we don't care about the warning/error, can we do better? Yes we can! We're writing the variable name twice, but passing by reference can help us here:

function default_value(&$var, $default) {
 if (empty($var)) {
 $var = $default;
 }
}
default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

It's much shorter, and feels more declarative: you can look at a bunch of default_value calls and see what the default values are instantly. But we still have the same warning/error issue.

Before dealing with your question, I'd like to point out that you probably want to use empty() instead of comparing with an empty string and that if you really want to compare to an empty string, use the === operator.

Anyway, default initialization is a real problem in many languages. C# even has a ?? operator to solve this kind of issue! So, let's try to make it better to make your code better step by step.

Ternary operator no more

The one-liner is too long, and using the ternary operator on multiple lines is a bad idea: that's what if-statement are for:

if (empty($newsItems[0]['image_url'])) {
 $newsItems[0]['image_url'] = '/img/cat_placeholder.jpg';
}

This is more readable for two reasons : it's easier to understand the syntax, but more importantly, it's easier to see that you simply want a default value when there's nothing. We don't need the 'else' part which is confusing with the ternary operator.

Note: as mentioned by Simon Scarfe and corrected by mseancole, PHP also has some special ternary syntax to do this:

$newsItems[0]['image_url'] = $newsItems[0]['image_url'] ?: 'img/cat_placeholder.jpg';

Factorize it!

If you're doing this only once or twice, then all is good, but otherwise you'd want to factorize it into a function, since you don't repeat yourself, right? The simple way is:

function default_value($var, $default) {
 return empty($var) ? $default : $var;
}
$newsItems[0]['image_url'] = default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

(The ternary operator does make sense here since variable names are short and both branches of the condition are useful.)

However, we're looking up $newsItems[0]['image_url'] when calling default_value, and this is possibly not defined, and will raise an error/warning. If that's a concern (it should), stick to the first version, or look at this other answer that gives a more robust solution at the expense of storing PHP code as a a string and thus cannot be checked syntactically.

Still too long

If we don't care about the warning/error, can we do better? Yes we can! We're writing the variable name twice, but passing by reference can help us here:

function default_value(&$var, $default) {
 if (empty($var)) {
 $var = $default;
 }
}
default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

It's much shorter, and feels more declarative: you can look at a bunch of default_value calls and see what the default values are instantly. But we still have the same warning/error issue.

Before dealing with your question, I'd like to point out that you probably want to use empty() instead of comparing with an empty string and that if you really want to compare to an empty string, use the === operator.

Anyway, default initialization is a real problem in many languages. C# even has a ?? operator to solve this kind of issue! So, let's try to make it better to make your code better step by step.

Ternary operator no more

The one-liner is too long, and using the ternary operator on multiple lines is a bad idea: that's what if-statement are for:

if (empty($newsItems[0]['image_url'])) {
 $newsItems[0]['image_url'] = '/img/cat_placeholder.jpg';
}

This is more readable for two reasons : it's easier to understand the syntax, but more importantly, it's easier to see that you simply want a default value when there's nothing. We don't need the 'else' part which is confusing with the ternary operator.

Note: as mentioned by Simon Scarfe and corrected by mseancole, PHP also has some special ternary syntax to do this:

$newsItems[0]['image_url'] = $newsItems[0]['image_url'] ?: 'img/cat_placeholder.jpg';

Factorize it!

If you're doing this only once or twice, then all is good, but otherwise you'd want to factorize it into a function, since you don't repeat yourself, right? The simple way is:

function default_value($var, $default) {
 return empty($var) ? $default : $var;
}
$newsItems[0]['image_url'] = default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

(The ternary operator does make sense here since variable names are short and both branches of the condition are useful.)

However, we're looking up $newsItems[0]['image_url'] when calling default_value, and this is possibly not defined, and will raise an error/warning. If that's a concern (it should), stick to the first version, or look at this other answer that gives a more robust solution at the expense of storing PHP code as a a string and thus cannot be checked syntactically.

Still too long

If we don't care about the warning/error, can we do better? Yes we can! We're writing the variable name twice, but passing by reference can help us here:

function default_value(&$var, $default) {
 if (empty($var)) {
 $var = $default;
 }
}
default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

It's much shorter, and feels more declarative: you can look at a bunch of default_value calls and see what the default values are instantly. But we still have the same warning/error issue.

Explain why Micheal J Mulligan is right, basically :)
Source Link
Quentin Pradet
  • 7.1k
  • 1
  • 25
  • 44

Before dealing with your question, I'd like to point out that you probably want to use empty() instead of comparing with an empty string and that if you really want to compare to an empty string, use the === operator.

Anyway, default initialization is a real problem in many languages. C# even has a ?? operator to solve this kind of issue! So, let's try to make it better to make your code better step by step.

Ternary operator no more

The one-liner is too long, and using the ternary operator on multiple lines is a bad idea: that's what if-statement are for:

if (empty($newsItems[0]['image_url'])) {
 $newsItems[0]['image_url'] = '/img/cat_placeholder.jpg';
}

This is more readable for two reasons : it's easier to understand the syntax, but more importantly, it's easier to see that you simply want a default value when there's nothing. We don't need the 'else' part which is confusing with the ternary operator.

Note: as mentioned by Simon Scarfe and corrected by mseancole, PHP also has some special ternary syntax to do this:

$newsItems[0]['image_url'] = $newsItems[0]['image_url'] ?: 'img/cat_placeholder.jpg';

Factorize it!

If you're doing this only once or twice, then all is good, but otherwise you'd want to factorize it into a function, since you don't repeat yourself, right? The simple way is:

function default_value($var, $default) {
 return empty($var) ? $default : $var;
}
$newsItems[0]['image_url'] = default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

(The ternary operator does make sense here since variable names are short and both branches of the condition are useful.)

However, we're looking up $newsItems[0]['image_url'] when calling default_value, and this is possibly not defined, and will raise an error/warning. If that's a concern (it should), stick to the first version, or look at this other answer that gives a more robust solution at the expense of storing PHP code as a a string and thus cannot be checked syntactically.

Still too long

CanIf we don't care about the warning/error, can we do better? Yes we can! We're writing the variable name twice, but passing by reference can help us here:

function default_value(&$var, $default) {
 if (empty($var)) {
 $var = $default;
 }
}
default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

It's much shorter, and feels more declarative: you can look at a bunch of default_value calls and see what the default values are instantly. But we still have the same warning/error issue.

Before dealing with your question, I'd like to point out that you probably want to use empty() instead of comparing with an empty string and that if you really want to compare to an empty string, use the === operator.

Anyway, default initialization is a real problem in many languages. C# even has a ?? operator to solve this kind of issue! So, let's try to make it better to make your code better step by step.

Ternary operator no more

The one-liner is too long, and using the ternary operator on multiple lines is a bad idea: that's what if-statement are for:

if (empty($newsItems[0]['image_url'])) {
 $newsItems[0]['image_url'] = '/img/cat_placeholder.jpg';
}

This is more readable for two reasons : it's easier to understand the syntax, but more importantly, it's easier to see that you simply want a default value when there's nothing. We don't need the 'else' part which is confusing with the ternary operator.

Note: as mentioned by Simon Scarfe and corrected by mseancole, PHP also has some special ternary syntax to do this:

$newsItems[0]['image_url'] = $newsItems[0]['image_url'] ?: 'img/cat_placeholder.jpg';

Factorize it!

If you're doing this only once or twice, then all is good, but otherwise you'd want to factorize it into a function, since you don't repeat yourself, right? The simple way is:

function default_value($var, $default) {
 return empty($var) ? $default : $var;
}
$newsItems[0]['image_url'] = default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

(The ternary operator does make sense here since variable names are short and both branches of the condition are useful.)

Still too long

Can we do better? Yes we can! We're writing the variable name twice, but passing by reference can help us here:

function default_value(&$var, $default) {
 if (empty($var)) {
 $var = $default;
 }
}
default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

It's much shorter, and feels more declarative: you can look at a bunch of default_value calls and see what the default values are instantly.

Before dealing with your question, I'd like to point out that you probably want to use empty() instead of comparing with an empty string and that if you really want to compare to an empty string, use the === operator.

Anyway, default initialization is a real problem in many languages. C# even has a ?? operator to solve this kind of issue! So, let's try to make it better to make your code better step by step.

Ternary operator no more

The one-liner is too long, and using the ternary operator on multiple lines is a bad idea: that's what if-statement are for:

if (empty($newsItems[0]['image_url'])) {
 $newsItems[0]['image_url'] = '/img/cat_placeholder.jpg';
}

This is more readable for two reasons : it's easier to understand the syntax, but more importantly, it's easier to see that you simply want a default value when there's nothing. We don't need the 'else' part which is confusing with the ternary operator.

Note: as mentioned by Simon Scarfe and corrected by mseancole, PHP also has some special ternary syntax to do this:

$newsItems[0]['image_url'] = $newsItems[0]['image_url'] ?: 'img/cat_placeholder.jpg';

Factorize it!

If you're doing this only once or twice, then all is good, but otherwise you'd want to factorize it into a function, since you don't repeat yourself, right? The simple way is:

function default_value($var, $default) {
 return empty($var) ? $default : $var;
}
$newsItems[0]['image_url'] = default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

(The ternary operator does make sense here since variable names are short and both branches of the condition are useful.)

However, we're looking up $newsItems[0]['image_url'] when calling default_value, and this is possibly not defined, and will raise an error/warning. If that's a concern (it should), stick to the first version, or look at this other answer that gives a more robust solution at the expense of storing PHP code as a a string and thus cannot be checked syntactically.

Still too long

If we don't care about the warning/error, can we do better? Yes we can! We're writing the variable name twice, but passing by reference can help us here:

function default_value(&$var, $default) {
 if (empty($var)) {
 $var = $default;
 }
}
default_value($newsItems[0]['image_url'], '/img/cat_placeholder.jpg');

It's much shorter, and feels more declarative: you can look at a bunch of default_value calls and see what the default values are instantly. But we still have the same warning/error issue.

shorthand ternary was wrong.
Source Link
Quentin Pradet
  • 7.1k
  • 1
  • 25
  • 44
Loading
?: syntax
Source Link
Quentin Pradet
  • 7.1k
  • 1
  • 25
  • 44
Loading
Thanks to @JaredCobb - there was an erroneous "!".
Source Link
Quentin Pradet
  • 7.1k
  • 1
  • 25
  • 44
Loading
Source Link
Quentin Pradet
  • 7.1k
  • 1
  • 25
  • 44
Loading
lang-php

AltStyle によって変換されたページ (->オリジナル) /