Skip to main content
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

Code review

There are too many things going on in this line:

if( @is_file($file = basename(__FILE__, '.css.php') . '-config.php' ) )
  • Formulate a file path from __FILE__, chopping off .css.php and appending -config.php
  • Store the path in a variable
  • Check if the path exists

It's hard to read when a single line does so much. It's better to break this up to two statements:

$file = basename(__FILE__, '.css.php') . '-config.php';
if (is_file($file)) {

The code is full of tedious isset($config['x']) && $config['x'] checks. To make that simpler, I would start with an array of default values, and copy over the parameter array, which will ensure that all values exist, so that you could skip all the isset checks.


I wonder what all the unset statements are about. I checked a few, and they don't seem to matter, as the values they unset are either not used later, or reassigned. They seem pointless (and paranoid), so you can remove them.

More reasons why this is a bad idea

@cimmanon@cimmanon mention some reasons, for me these are the biggest:

  • The main objective of utility code should be to reduce complexity. This code, unfortunately, is doing the opposite, by adding another API layer for working with styles. CSS alone is hard enough to figure out and debug. To use your generator, I have to learn another API (your configuration logic), and if something doesn't work as expected, I may have to debug this extra API in addition to plain old CSS.

  • If you want to generate CSS, look into frameworks designed for this, for example SASS, LESS, or one of the many other similar tools that are new and sexy and I've never even heard about. Like your tool, these too add another API layer, but that's ok, because they add massive value on top of CSS.

  • I see some usability issues. Let's say you have 3 pages that use slightly different style sheets. How will that work with this script? It seems you would need 3 copies of the script. Or better, use symlinks. Even so, it seems tedious compared using CSS alone and no extra PHP files per custom style.

  • CSS files can be zipped, which can be very important for sites where performance is paramount. (The frameworks I mentioned earlier help with that too.) This is in contrast with CSS generated on the fly. To get that zipped, you'd probably have to save to a file first, which is yet another hoop to jump through.

Code review

There are too many things going on in this line:

if( @is_file($file = basename(__FILE__, '.css.php') . '-config.php' ) )
  • Formulate a file path from __FILE__, chopping off .css.php and appending -config.php
  • Store the path in a variable
  • Check if the path exists

It's hard to read when a single line does so much. It's better to break this up to two statements:

$file = basename(__FILE__, '.css.php') . '-config.php';
if (is_file($file)) {

The code is full of tedious isset($config['x']) && $config['x'] checks. To make that simpler, I would start with an array of default values, and copy over the parameter array, which will ensure that all values exist, so that you could skip all the isset checks.


I wonder what all the unset statements are about. I checked a few, and they don't seem to matter, as the values they unset are either not used later, or reassigned. They seem pointless (and paranoid), so you can remove them.

More reasons why this is a bad idea

@cimmanon mention some reasons, for me these are the biggest:

  • The main objective of utility code should be to reduce complexity. This code, unfortunately, is doing the opposite, by adding another API layer for working with styles. CSS alone is hard enough to figure out and debug. To use your generator, I have to learn another API (your configuration logic), and if something doesn't work as expected, I may have to debug this extra API in addition to plain old CSS.

  • If you want to generate CSS, look into frameworks designed for this, for example SASS, LESS, or one of the many other similar tools that are new and sexy and I've never even heard about. Like your tool, these too add another API layer, but that's ok, because they add massive value on top of CSS.

  • I see some usability issues. Let's say you have 3 pages that use slightly different style sheets. How will that work with this script? It seems you would need 3 copies of the script. Or better, use symlinks. Even so, it seems tedious compared using CSS alone and no extra PHP files per custom style.

  • CSS files can be zipped, which can be very important for sites where performance is paramount. (The frameworks I mentioned earlier help with that too.) This is in contrast with CSS generated on the fly. To get that zipped, you'd probably have to save to a file first, which is yet another hoop to jump through.

Code review

There are too many things going on in this line:

if( @is_file($file = basename(__FILE__, '.css.php') . '-config.php' ) )
  • Formulate a file path from __FILE__, chopping off .css.php and appending -config.php
  • Store the path in a variable
  • Check if the path exists

It's hard to read when a single line does so much. It's better to break this up to two statements:

$file = basename(__FILE__, '.css.php') . '-config.php';
if (is_file($file)) {

The code is full of tedious isset($config['x']) && $config['x'] checks. To make that simpler, I would start with an array of default values, and copy over the parameter array, which will ensure that all values exist, so that you could skip all the isset checks.


I wonder what all the unset statements are about. I checked a few, and they don't seem to matter, as the values they unset are either not used later, or reassigned. They seem pointless (and paranoid), so you can remove them.

More reasons why this is a bad idea

@cimmanon mention some reasons, for me these are the biggest:

  • The main objective of utility code should be to reduce complexity. This code, unfortunately, is doing the opposite, by adding another API layer for working with styles. CSS alone is hard enough to figure out and debug. To use your generator, I have to learn another API (your configuration logic), and if something doesn't work as expected, I may have to debug this extra API in addition to plain old CSS.

  • If you want to generate CSS, look into frameworks designed for this, for example SASS, LESS, or one of the many other similar tools that are new and sexy and I've never even heard about. Like your tool, these too add another API layer, but that's ok, because they add massive value on top of CSS.

  • I see some usability issues. Let's say you have 3 pages that use slightly different style sheets. How will that work with this script? It seems you would need 3 copies of the script. Or better, use symlinks. Even so, it seems tedious compared using CSS alone and no extra PHP files per custom style.

  • CSS files can be zipped, which can be very important for sites where performance is paramount. (The frameworks I mentioned earlier help with that too.) This is in contrast with CSS generated on the fly. To get that zipped, you'd probably have to save to a file first, which is yet another hoop to jump through.

added 59 characters in body
Source Link
janos
  • 111.7k
  • 15
  • 152
  • 391

Code review

There are too many things going on in this line:

if( @is_file($file = basename(__FILE__, '.css.php') . '-config.php' ) )
  • Formulate a file path from __FILE__, chopping off .css.php and appending -config.php
  • Store the path in a variable
  • Check if the path exists

It's hard to read when a single line does so much. It's better to break this up to two statements:

$file = basename(__FILE__, '.css.php') . '-config.php';
if (is_file($file)) {

The code is full of tedious isset($config['x']) && $config['x'] checks. To make that simpler, I would start with an array of default values, and copy over the parameter array, which will ensure that all values exist, so that you could skip all the isset checks.


I wonder what all the unset statements are about. I checked a few, and they don't seem to matter, as the values they unset are either not used later, or reassigned. They seem pointless (and paranoid), so you can remove them.

More reasons why this is a bad idea

@cimmanon mention some reasons, for me these are the biggest:

  • The main objective of utility code should be to reduce complexity. This code, unfortunately, is doing the opposite, by adding another API layer for working with styles. CSS alone is hard enough to figure out and debug. To use your generator, I have to learn another API (your configuration logic), and if something doesn't work as expected, I may have to debug this extra API in addition to plain old CSS.

  • If you want to generate CSS, look into frameworks designed for this, for example SASS, LESS, or one of the many other similar tools that are new and sexy and I've never even heard about. Like your tool, these too add another API layer, but that's ok, because they add massive value on top of CSS.

  • I see some usability issues. Let's say you have 3 pages that use slightly different style sheets. How will that work with this script? It seems you would need 3 copies of the script. Or better, use symlinks. Even so, it seems tedious compared using CSS alone and no extra PHP files per custom style.

  • CSS files can be zipped, which can be very important for sites where performance is paramount. (The frameworks I mentioned earlier help with that too.) This is in contrast with CSS generated by PHP, I'm not sure how to arrange for gettingon the fly. To get that zipped, you'd probably have to save to a file first, which is yet another hoop to jump through.

Code review

There are too many things going on in this line:

if( @is_file($file = basename(__FILE__, '.css.php') . '-config.php' ) )
  • Formulate a file path from __FILE__, chopping off .css.php and appending -config.php
  • Store the path in a variable
  • Check if the path exists

It's hard to read when a single line does so much. It's better to break this up to two statements:

$file = basename(__FILE__, '.css.php') . '-config.php';
if (is_file($file)) {

The code is full of tedious isset($config['x']) && $config['x'] checks. To make that simpler, I would start with an array of default values, and copy over the parameter array, which will ensure that all values exist, so that you could skip all the isset checks.


I wonder what all the unset statements are about. I checked a few, and they don't seem to matter, as the values they unset are either not used later, or reassigned. They seem pointless (and paranoid), so you can remove them.

More reasons why this is a bad idea

@cimmanon mention some reasons, for me these are the biggest:

  • The main objective of utility code should be to reduce complexity. This code, unfortunately, is doing the opposite, by adding another API layer for working with styles. CSS alone is hard enough to figure out and debug. To use your generator, I have to learn another API (your configuration logic), and if something doesn't work as expected, I may have to debug this extra API in addition to plain old CSS.

  • If you want to generate CSS, look into frameworks designed for this, for example SASS, LESS, or one of the many other similar tools that are new and sexy and I've never even heard about. Like your tool, these too add another API layer, but that's ok, because they add massive value on top of CSS.

  • I see some usability issues. Let's say you have 3 pages that use slightly different style sheets. How will that work with this script? It seems you would need 3 copies of the script. Or better, use symlinks. Even so, it seems tedious compared using CSS alone and no extra PHP files per custom style.

  • CSS files can be zipped, which can be very important for sites where performance is paramount. (The frameworks I mentioned earlier help with that too.) This is in contrast with CSS generated by PHP, I'm not sure how to arrange for getting that zipped.

Code review

There are too many things going on in this line:

if( @is_file($file = basename(__FILE__, '.css.php') . '-config.php' ) )
  • Formulate a file path from __FILE__, chopping off .css.php and appending -config.php
  • Store the path in a variable
  • Check if the path exists

It's hard to read when a single line does so much. It's better to break this up to two statements:

$file = basename(__FILE__, '.css.php') . '-config.php';
if (is_file($file)) {

The code is full of tedious isset($config['x']) && $config['x'] checks. To make that simpler, I would start with an array of default values, and copy over the parameter array, which will ensure that all values exist, so that you could skip all the isset checks.


I wonder what all the unset statements are about. I checked a few, and they don't seem to matter, as the values they unset are either not used later, or reassigned. They seem pointless (and paranoid), so you can remove them.

More reasons why this is a bad idea

@cimmanon mention some reasons, for me these are the biggest:

  • The main objective of utility code should be to reduce complexity. This code, unfortunately, is doing the opposite, by adding another API layer for working with styles. CSS alone is hard enough to figure out and debug. To use your generator, I have to learn another API (your configuration logic), and if something doesn't work as expected, I may have to debug this extra API in addition to plain old CSS.

  • If you want to generate CSS, look into frameworks designed for this, for example SASS, LESS, or one of the many other similar tools that are new and sexy and I've never even heard about. Like your tool, these too add another API layer, but that's ok, because they add massive value on top of CSS.

  • I see some usability issues. Let's say you have 3 pages that use slightly different style sheets. How will that work with this script? It seems you would need 3 copies of the script. Or better, use symlinks. Even so, it seems tedious compared using CSS alone and no extra PHP files per custom style.

  • CSS files can be zipped, which can be very important for sites where performance is paramount. (The frameworks I mentioned earlier help with that too.) This is in contrast with CSS generated on the fly. To get that zipped, you'd probably have to save to a file first, which is yet another hoop to jump through.

missing sssss
Source Link
rolfl
  • 97.5k
  • 17
  • 218
  • 418

Code review

There are too many things going on in this line:

if( @is_file($file = basename(__FILE__, '.css.php') . '-config.php' ) )
  • Formulate a file path from __FILE__, chopping off .css.php and appending -config.php
  • Store the path in a variable
  • Check if the path exists

It's hard to read when a single line does so much. It's better to break this up to two statements:

$file = basename(__FILE__, '.css.php') . '-config.php';
if (is_file($file)) {

The code is full of tedious isset($config['x']) && $config['x'] checks. To make that simpler, I would start with an array of default values, and copy over the parameter array, which will ensure that all values exitexist, so that you could skip all the isset checks.


I wonder what all the unset statements are about. I checked a few, and they don't seem to matter, as the values they unset are either not used later, or reassigned. They seem pointless (and paranoid), so you can remove them.

More reasons why this is a bad idea

@cimmanon mention some reasons, for me these are the biggest:

  • The main objective of utility code should be to reduce complexity. This code, unfortunately, is doing the opposite, by adding another API layer for working with styles. CSS alone is hard enough to figure out and debug. To use your generator, I have to learn another API (your configuration logic), and if something doesn't work as expected, I may have to debug this extra API in addition to plain old CSS.

  • If you want to generate CSS, look into frameworks designed for this, for example SASS, LESS, or one of the many other similar tools that are new and sexy and I've never even heard about. Like your tool, these too add another API layer, but that's ok, because they add massive value on top of CSS.

  • I see some usability issues. Let's say you have 3 pages that use slightly different style sheets. How will that work with this script? It seems you would need 3 copies of the script. Or better, use symlinks. Even so, it seems tedious compared using CSS alone and no extra PHP files per custom style.

  • CSS files can be zipped, which can be very important for sites where performance is paramount. (The frameworks I mentioned earlier help with that too.) This is in contrast with CSS generated by PHP, I'm not sure how to arrange for getting that zipped.

Code review

There are too many things going on in this line:

if( @is_file($file = basename(__FILE__, '.css.php') . '-config.php' ) )
  • Formulate a file path from __FILE__, chopping off .css.php and appending -config.php
  • Store the path in a variable
  • Check if the path exists

It's hard to read when a single line does so much. It's better to break this up to two statements:

$file = basename(__FILE__, '.css.php') . '-config.php';
if (is_file($file)) {

The code is full of tedious isset($config['x']) && $config['x'] checks. To make that simpler, I would start with an array of default values, and copy over the parameter array, which will ensure that all values exit, so that you could skip all the isset checks.


I wonder what all the unset statements are about. I checked a few, and they don't seem to matter, as the values they unset are either not used later, or reassigned. They seem pointless (and paranoid), so you can remove them.

More reasons why this is a bad idea

@cimmanon mention some reasons, for me these are the biggest:

  • The main objective of utility code should be to reduce complexity. This code, unfortunately, is doing the opposite, by adding another API layer for working with styles. CSS alone is hard enough to figure out and debug. To use your generator, I have to learn another API (your configuration logic), and if something doesn't work as expected, I may have to debug this extra API in addition to plain old CSS.

  • If you want to generate CSS, look into frameworks designed for this, for example SASS, LESS, or one of the many other similar tools that are new and sexy and I've never even heard about. Like your tool, these too add another API layer, but that's ok, because they add massive value on top of CSS.

  • I see some usability issues. Let's say you have 3 pages that use slightly different style sheets. How will that work with this script? It seems you would need 3 copies of the script. Or better, use symlinks. Even so, it seems tedious compared using CSS alone and no extra PHP files per custom style.

  • CSS files can be zipped, which can be very important for sites where performance is paramount. (The frameworks I mentioned earlier help with that too.) This is in contrast with CSS generated by PHP, I'm not sure how to arrange for getting that zipped.

Code review

There are too many things going on in this line:

if( @is_file($file = basename(__FILE__, '.css.php') . '-config.php' ) )
  • Formulate a file path from __FILE__, chopping off .css.php and appending -config.php
  • Store the path in a variable
  • Check if the path exists

It's hard to read when a single line does so much. It's better to break this up to two statements:

$file = basename(__FILE__, '.css.php') . '-config.php';
if (is_file($file)) {

The code is full of tedious isset($config['x']) && $config['x'] checks. To make that simpler, I would start with an array of default values, and copy over the parameter array, which will ensure that all values exist, so that you could skip all the isset checks.


I wonder what all the unset statements are about. I checked a few, and they don't seem to matter, as the values they unset are either not used later, or reassigned. They seem pointless (and paranoid), so you can remove them.

More reasons why this is a bad idea

@cimmanon mention some reasons, for me these are the biggest:

  • The main objective of utility code should be to reduce complexity. This code, unfortunately, is doing the opposite, by adding another API layer for working with styles. CSS alone is hard enough to figure out and debug. To use your generator, I have to learn another API (your configuration logic), and if something doesn't work as expected, I may have to debug this extra API in addition to plain old CSS.

  • If you want to generate CSS, look into frameworks designed for this, for example SASS, LESS, or one of the many other similar tools that are new and sexy and I've never even heard about. Like your tool, these too add another API layer, but that's ok, because they add massive value on top of CSS.

  • I see some usability issues. Let's say you have 3 pages that use slightly different style sheets. How will that work with this script? It seems you would need 3 copies of the script. Or better, use symlinks. Even so, it seems tedious compared using CSS alone and no extra PHP files per custom style.

  • CSS files can be zipped, which can be very important for sites where performance is paramount. (The frameworks I mentioned earlier help with that too.) This is in contrast with CSS generated by PHP, I'm not sure how to arrange for getting that zipped.

deleted 85 characters in body
Source Link
janos
  • 111.7k
  • 15
  • 152
  • 391
Loading
Source Link
janos
  • 111.7k
  • 15
  • 152
  • 391
Loading