Skip to main content
added 853 characters in body
Source Link
Jayy
  • 340
  • 1
  • 12

There is nothing inherently wrong with foreach loops.

Alternatives might include:

  • Creating objects that contain the data and extend Iterator, and using that to recurse the options. The disadvantages are that it would be less efficient and would involve quite a lot more code. The advantage is it would make the foreach loops more readable; particularly whats inside them, once you give methods to each of the objects that extend Iterator that perform the action that happens within the foreach code.
  • use PHP array functions. This is probably the most efficient, but can make code painfully unreadable. I would not recommend this unless you desperately need greater efficiency.

By the way, one change I would make no matter what you do, is replace "magic constants" with named constants. E.g.:

if ( in_array( $menu_item[2], $new_menu_items ) ) {

becomes

define ('MEANINGFUL_NAME', 2);

// ....

if ( in_array( $menu_item[MEANINGFUL_NAME], $new_menu_items ) ) {

Otherwise, when you come back to look at the code after a long time, you will have no idea if each time you see a 2, whether they all refer to the same "thing" or not; in other words, say you want to change an aspect of it to be "3" instead, you will have to painstakingly work out what each bit of code does to see if it is a 2 that you need to change or not. (Using comments is a bad idea for this, as comments are easily forgotten to be changed, and it would have to be commented each time you have a magic number - the more you have, the more likely changes in code will not have corresponding comments changed

One final change I would make is perhaps splitting up the function into sub-functions, since doing that would mean you can give the functions names that describe the sub-process being performed. Putting that method and sub methods into an appropriate class could nicely tidy things up. While OOP can lead (IMO) to the very best code, if it's implemented badly it can be a nightmare to manage. Plenty of books on it, which I could recommend if you like. I have 5-10 books on it that I always keep by my side.

  • "Design Patterns" Gamma / Helm / Jhnson / Vlissides (1995)
  • "Patterns of Enterprise Application Architecture" Martin Fowler (2003)
  • "The pragmatic programmer" Hunt / Thomas (2000)
  • "Refactoring to patterns" Kerievsky (2005)
  • "Refactoring" - Martin Fowler (1999)
  • "OO Design Heuristics" - Arthur Riel (1996)
  • "Growing Object-Oriented Software Guided by Tests" - Freeman / Pryce (2009)

I would actually suggest first getting Freeman/Pryce, even if you find TDD doesn't really agree with you - their bibliography was very useful to me, and I picked and choose what suited me. There's a lot of controversy about TDD, but they'll show you why refactoring is such a powerful principle, and you can then try one or two books on patterns and refactoring. Choose one that seems to have patterns you think you'll be most likely to use.

There is nothing inherently wrong with foreach loops.

Alternatives might include:

  • Creating objects that contain the data and extend Iterator, and using that to recurse the options. The disadvantages are that it would be less efficient and would involve quite a lot more code. The advantage is it would make the foreach loops more readable; particularly whats inside them, once you give methods to each of the objects that extend Iterator that perform the action that happens within the foreach code.
  • use PHP array functions. This is probably the most efficient, but can make code painfully unreadable. I would not recommend this unless you desperately need greater efficiency.

By the way, one change I would make no matter what you do, is replace "magic constants" with named constants. E.g.:

if ( in_array( $menu_item[2], $new_menu_items ) ) {

becomes

define ('MEANINGFUL_NAME', 2);

// ....

if ( in_array( $menu_item[MEANINGFUL_NAME], $new_menu_items ) ) {

Otherwise, when you come back to look at the code after a long time, you will have no idea if each time you see a 2, whether they all refer to the same "thing" or not; in other words, say you want to change an aspect of it to be "3" instead, you will have to painstakingly work out what each bit of code does to see if it is a 2 that you need to change or not. (Using comments is a bad idea for this, as comments are easily forgotten to be changed, and it would have to be commented each time you have a magic number - the more you have, the more likely changes in code will not have corresponding comments changed

One final change I would make is perhaps splitting up the function into sub-functions, since doing that would mean you can give the functions names that describe the sub-process being performed. Putting that method and sub methods into an appropriate class could nicely tidy things up. While OOP can lead (IMO) to the very best code, if it's implemented badly it can be a nightmare to manage. Plenty of books on it, which I could recommend if you like. I have 5-10 books on it that I always keep by my side.

There is nothing inherently wrong with foreach loops.

Alternatives might include:

  • Creating objects that contain the data and extend Iterator, and using that to recurse the options. The disadvantages are that it would be less efficient and would involve quite a lot more code. The advantage is it would make the foreach loops more readable; particularly whats inside them, once you give methods to each of the objects that extend Iterator that perform the action that happens within the foreach code.
  • use PHP array functions. This is probably the most efficient, but can make code painfully unreadable. I would not recommend this unless you desperately need greater efficiency.

By the way, one change I would make no matter what you do, is replace "magic constants" with named constants. E.g.:

if ( in_array( $menu_item[2], $new_menu_items ) ) {

becomes

define ('MEANINGFUL_NAME', 2);

// ....

if ( in_array( $menu_item[MEANINGFUL_NAME], $new_menu_items ) ) {

Otherwise, when you come back to look at the code after a long time, you will have no idea if each time you see a 2, whether they all refer to the same "thing" or not; in other words, say you want to change an aspect of it to be "3" instead, you will have to painstakingly work out what each bit of code does to see if it is a 2 that you need to change or not. (Using comments is a bad idea for this, as comments are easily forgotten to be changed, and it would have to be commented each time you have a magic number - the more you have, the more likely changes in code will not have corresponding comments changed

One final change I would make is perhaps splitting up the function into sub-functions, since doing that would mean you can give the functions names that describe the sub-process being performed. Putting that method and sub methods into an appropriate class could nicely tidy things up. While OOP can lead (IMO) to the very best code, if it's implemented badly it can be a nightmare to manage. Plenty of books on it, which I could recommend if you like. I have 5-10 books on it that I always keep by my side.

  • "Design Patterns" Gamma / Helm / Jhnson / Vlissides (1995)
  • "Patterns of Enterprise Application Architecture" Martin Fowler (2003)
  • "The pragmatic programmer" Hunt / Thomas (2000)
  • "Refactoring to patterns" Kerievsky (2005)
  • "Refactoring" - Martin Fowler (1999)
  • "OO Design Heuristics" - Arthur Riel (1996)
  • "Growing Object-Oriented Software Guided by Tests" - Freeman / Pryce (2009)

I would actually suggest first getting Freeman/Pryce, even if you find TDD doesn't really agree with you - their bibliography was very useful to me, and I picked and choose what suited me. There's a lot of controversy about TDD, but they'll show you why refactoring is such a powerful principle, and you can then try one or two books on patterns and refactoring. Choose one that seems to have patterns you think you'll be most likely to use.

added 398 characters in body
Source Link
Jayy
  • 340
  • 1
  • 12

There is nothing inherently wrong with foreach loops.

Alternatives might include:

  • Creating objects that contain the data and extend iteratorableIterator, and using that to recurse the options. The disadvantages are that it would be less efficient and would involve quite a lot more code. The advantage is it would make the foreach loops more readable; particularly whats inside them, once you give methods to each of the objects that extend iteratable methodsIterator that perform the action that happens within the foreach code.
  • use PHP array functions. This is probably the most efficient, but can make code painfully unreadable. I would not recommend this unless you desperately need greater efficiency.

By the way, one change I would make no matter what you do, is replace "magic constants" with named constants. E.g.:

if ( in_array( $menu_item[2], $new_menu_items ) ) {

becomes

define ('MEANINGFUL_NAME', 2);

// ....

if ( in_array( $menu_item[MEANINGFUL_NAME], $new_menu_items ) ) {

Otherwise, when you come back to look at the code after a long time, you will have no idea if each time you see a 2, whether they all refer to the same "thing" or not; in other words, say you want to change an aspect of it to be "3" instead, you will have to painstakingly work out what each bit of code does to see if it is a 2 that you need to change or not. (Using comments is a bad idea for this, as comments are easily forgotten to be changed, and it would have to be commented each time you have a magic number - the more you have, the more likely changes in code will not have corresponding comments changed

One final change I would make is perhaps splitting up the function into sub-functions, since doing that would mean you can give the functions names that describe the sub-process being performed. Putting that method and sub methods into an appropriate class could nicely tidy things up. While OOP can lead (IMO) to the very best code, if it's implemented badly it can be a nightmare to manage. Plenty of books on it, which I could recommend if you like. I have 5-10 books on it that I always keep by my side.

There is nothing inherently wrong with foreach loops.

Alternatives might include:

  • Creating objects that contain the data and extend iteratorable, and using that to recurse the options. The disadvantages are that it would be less efficient and would involve quite a lot more code. The advantage is it would make the foreach loops more readable; particularly whats inside them, once you give each of the objects that extend iteratable methods that perform the action that happens within the foreach code.
  • use PHP array functions. This is probably the most efficient, but can make code painfully unreadable. I would not recommend this unless you desperately need greater efficiency.

By the way, one change I would make no matter what you do, is replace "magic constants" with named constants. E.g.:

if ( in_array( $menu_item[2], $new_menu_items ) ) {

becomes

define ('MEANINGFUL_NAME', 2);

// ....

if ( in_array( $menu_item[MEANINGFUL_NAME], $new_menu_items ) ) {

Otherwise, when you come back to look at the code after a long time, you will have no idea if each time you see a 2, whether they all refer to the same "thing" or not; in other words, say you want to change an aspect of it to be "3" instead, you will have to painstakingly work out what each bit of code does to see if it is a 2 that you need to change or not. (Using comments is a bad idea for this, as comments are easily forgotten to be changed, and it would have to be commented each time you have a magic number - the more you have, the more likely changes in code will not have corresponding comments changed

There is nothing inherently wrong with foreach loops.

Alternatives might include:

  • Creating objects that contain the data and extend Iterator, and using that to recurse the options. The disadvantages are that it would be less efficient and would involve quite a lot more code. The advantage is it would make the foreach loops more readable; particularly whats inside them, once you give methods to each of the objects that extend Iterator that perform the action that happens within the foreach code.
  • use PHP array functions. This is probably the most efficient, but can make code painfully unreadable. I would not recommend this unless you desperately need greater efficiency.

By the way, one change I would make no matter what you do, is replace "magic constants" with named constants. E.g.:

if ( in_array( $menu_item[2], $new_menu_items ) ) {

becomes

define ('MEANINGFUL_NAME', 2);

// ....

if ( in_array( $menu_item[MEANINGFUL_NAME], $new_menu_items ) ) {

Otherwise, when you come back to look at the code after a long time, you will have no idea if each time you see a 2, whether they all refer to the same "thing" or not; in other words, say you want to change an aspect of it to be "3" instead, you will have to painstakingly work out what each bit of code does to see if it is a 2 that you need to change or not. (Using comments is a bad idea for this, as comments are easily forgotten to be changed, and it would have to be commented each time you have a magic number - the more you have, the more likely changes in code will not have corresponding comments changed

One final change I would make is perhaps splitting up the function into sub-functions, since doing that would mean you can give the functions names that describe the sub-process being performed. Putting that method and sub methods into an appropriate class could nicely tidy things up. While OOP can lead (IMO) to the very best code, if it's implemented badly it can be a nightmare to manage. Plenty of books on it, which I could recommend if you like. I have 5-10 books on it that I always keep by my side.

Source Link
Jayy
  • 340
  • 1
  • 12

There is nothing inherently wrong with foreach loops.

Alternatives might include:

  • Creating objects that contain the data and extend iteratorable, and using that to recurse the options. The disadvantages are that it would be less efficient and would involve quite a lot more code. The advantage is it would make the foreach loops more readable; particularly whats inside them, once you give each of the objects that extend iteratable methods that perform the action that happens within the foreach code.
  • use PHP array functions. This is probably the most efficient, but can make code painfully unreadable. I would not recommend this unless you desperately need greater efficiency.

By the way, one change I would make no matter what you do, is replace "magic constants" with named constants. E.g.:

if ( in_array( $menu_item[2], $new_menu_items ) ) {

becomes

define ('MEANINGFUL_NAME', 2);

// ....

if ( in_array( $menu_item[MEANINGFUL_NAME], $new_menu_items ) ) {

Otherwise, when you come back to look at the code after a long time, you will have no idea if each time you see a 2, whether they all refer to the same "thing" or not; in other words, say you want to change an aspect of it to be "3" instead, you will have to painstakingly work out what each bit of code does to see if it is a 2 that you need to change or not. (Using comments is a bad idea for this, as comments are easily forgotten to be changed, and it would have to be commented each time you have a magic number - the more you have, the more likely changes in code will not have corresponding comments changed