Opened 9 years ago

Closed 9 years ago

#1488 closed New Feature (fixed)

new php5 connector

Reported by: tedivm Owned by:
Priority: Normal Milestone: FCKeditor 2.6
Component: Server : PHP Version:
Keywords: Confirmed HasPatch Cc:

Description

Description of changes:

Continued with the update to proper php5

  • Removed depreciated 'var' tag from properties, added proper keywords.
  • Cleaned up the User Agent code
  • Separated out the public methods from the helper methods

Removed EncodeConfig method and replaced all occurrences of it with the php urlencode function

Optimized how the strings were formed (concatenating as opposed to embedding, which tends to run slower)

Refractored some odd if/else conditionals to use the ternary conditional instead

Added an 'ID' property so developers can specify the ID instead of using the InstanceName

The biggest thing about this is that I made changes so it matched the PHP5 OOP structure. PHP5 is backwards compatible with PHP4, so this class still worked, but work on PHP6 is underway and I doubt it will be backwards compatible all the way back to PHP4 so I brought everything up to date.

There were a couple of other changes, mostly to just make more efficient use of PHP. All the methods still return the same information (except for EncodeConfig, which wasn't really called by the public anyways). Default behavior is still the same- if a developer doesn't specify the ID, for instance, it still defaults to the InstanceName. I was able to drop the file into a couple of different places where I've used FCKeditor without having any issues.

Attachments (1)

fckeditor_php5.php (4.8 KB) - added by tedivm 9 years ago.

Download all attachments as: .zip

Change History (7)

Changed 9 years ago by tedivm

comment:1 Changed 9 years ago by fredck

  • Milestone set to FCKeditor 2.6

comment:2 Changed 9 years ago by w.olchawa

  • Keywords Confirmed HasPatch added

comment:3 Changed 9 years ago by wwalc

  • Resolution set to fixed
  • Status changed from new to closed

Thanks, fixed with [1577].

PHP6 compatibility is important, however changing anything from public to protected may potentially break existing applications, so I have set the visibility to public everywhere. Ternary conditional makes the code shorter, but also sometimes makes it harder to read.

comment:4 Changed 9 years ago by wwalc

...and [1578].

comment:5 Changed 9 years ago by tedivm

  • Resolution fixed deleted
  • Status changed from closed to reopened

wwalc- I didn't try to make it compatible with PHP6, I just tried to make it compatible with php5 and commented on the fact that, since php6 is coming out soon, maybe php5 compatibility is something to reach for. Whether things are public or protected doesn't matter, as long as they have something.

When you made your changes, you undid a lot of the work I put into the code. Specifically:

1) Your code doesn't allow developers to customize the ID field, which mine did (and is kind of important). Before you comment on backwards compatibility, I set it so that it would behave like it used to UNLESS someone specifically overrides the ID.

2) You removed the edits I made to the string concatenation. For instance

$Link = $this->BasePath . 'editor/' .$File . '?InstanceName=' . $this->InstanceName; you turned back into: $Link = "{$this->BasePath}editor/{$File}?InstanceName={$this->InstanceName}" ;

For someone unfamiliar with PHP, this might not seem like a bit deal. However, the way I did it is not only closer to the standard way, but is also significantly faster. It also uses up a lot more memory, so much so that PHP specifically comments about it in their manual- "Note: Parsing variables within strings uses more memory than string concatenation. When writing a PHP script in which memory usage is a concern, consider using the concatenation operator (.) rather than variable parsing. "

3) Ternary statements were specifically used because they make things easier to read and run faster.

As I mentioned before, I would be happy to assist in developing this out further. This file is also in use in a few dozen sites now, and it works fine.

comment:6 Changed 9 years ago by wwalc

  • Resolution set to fixed
  • Status changed from reopened to closed

Tedvim, I'm very happy that you want to help us, however let me explain why I've decided not to include all the proposed changes.

Whether things are public or protected does matter, because if someone used the IsCompatible() method in his application, now it will fail because of the "protected" keyword.

Customizing the ID field - please create a new ticket for it, explain why it is so important, so that we could all discuss this new functionality, instead of simply adding it to the PHP connector. If we all agree that it is something really needed, we will add this to all connectors, not only to that specific for PHP5.

Speed/performace - please note that this script has only around 200 lines. It is not running a million times in a loop. Changing if statements to ternary statements just because someone wrote they are faster doesn't make sense. They are faster, but if you run them 1000000 times, you save half a second. So unless you're in a loop, it's just a matter of coding preferences. Both ways of string concatenation and writing conditional statements are valid.

So the key point in changes I applied, was to make it E_STRICT compatible with older PHP5 versions (<5.1.3) and prepare for PHP6.

Note: See TracTickets for help on using tickets.
© 2003 – 2016 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy