caxy / php-htmldiff

A library for comparing two HTML files/snippets and highlighting the differences using simple HTML. Includes support for comparing complex lists and tables
http://php-htmldiff.caxy.com
GNU General Public License v2.0
202 stars 51 forks source link

Ported ListDiff to DOMDocument #96

Closed SavageTiger closed 3 years ago

SavageTiger commented 3 years ago

Description

As a preparation to move away from string manipulation (HtmlDiff) and having to ping pong between DOMDocument (TableDiff) / SimpeXML + extended library (ListDiff), I have ported the ListDiff to also use DOMDocument.

We can now get rid of the third party php-simple-html-dom-parser.

Huge string to dom overhead

We have huge overhead now in both ListDiff and TableDiff where we keep going from an html string to objects and back, we should get rid of this once HtmlDiff is also ported

Performance impact

I have been doing loads of testing, and the performance impact is minimal, it turns out it is slightly faster.

In some special cases it can even be 30% faster, but that is not in really realistic scenario's

Big cleanup

All the old ListDiff code was still in the codebase, but none of it was used since it was replaced by HtmlDiffList so that was a huge easy cleanup

jschroed91 commented 3 years ago

Really liking these improvements here @SavageTiger ! I haven't had a chance to review in depth, and don't let my lack of a review be a blocker to you if you're confident in the work.

SavageTiger commented 3 years ago

@jschroed91 I know you trust me to do the right thing and believe me, I have been doing loads of testing, and found loads of tiny bugs in the process haha

Just invited you as a reviewer more as a celebratory moment, that some cool improvements have been done again.

jschroed91 commented 3 years ago

@SavageTiger Yes this is AWESOME! It sounds like huge improvements without any noticeable tradeoffs, which is great. So glad that you've found and squashes some pesky bugs, and being able to drop an entire dependency of another library will be a great bonus.