Repairing BASE64 encoded vCard version 3

[Webbeh]

Follow up of
#415

Repairs vCards in V3 that have a BASE64 encoding.

[codecov]

Codecov Report

Merging #441 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master     #441      +/-   ##
============================================
+ Coverage     98.87%   98.87%   +<.01%     
- Complexity     1809     1811       +2     
============================================
  Files            65       65              
  Lines          5160     5168       +8     
============================================
+ Hits           5102     5110       +8     
  Misses           58       58

Impacted Files	Coverage Δ	Complexity Δ
lib/Property.php	98.96% <100%> (+0.04%)	70 <0> (+2)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b86533...24bb625. Read the comment docs.

[evert]

Looks great. Thank you very much. I added 2 tiny tweaks that you can accept right from the review (if you want them).

[Webbeh]

What's the point of the "offsetSet" function if you're not using it ? Just wondering.

Also the identity operator shouldn't be necessary in this case, as strtoupper() always returns a string, but I guess it doesn't hurt.

Applying the changes. Do you think we can see this merged in anytime soon ?

[evert]

What's the point of the "offsetSet" function if you're not using it ? Just wondering.

The offsetSet function is part of the ArrayAccess PHP interface. This interface's sole role is to make sure that $obj['item'] can be used.

Also the identity operator shouldn't be necessary in this case, as strtoupper() always returns a string, but I guess it doesn't hurt.

Apparently it's slightly faster, but it's probably negligible. As a general rule I find that using string comparisons (=== and !==) should be used always, except if you deliberately want to opt-out of strict checking and must use == for specific situations. Applying this rule strictly can avoid subtle bugs.

In this case neither of my suggestions actually did anything, it's just small stuff to help guard consistency. I also never used the 'suggest' feature before so I hoped you could just accept the suggestion with the click of a button ;)

[Webbeh]

It was not just "one click" but close enough :P

Did you have to override the offsetSet method ? I don't see what more it could do compared to standard arrays.

Yeah I know, it was just for consistency. It makes sense than the identity operator is slightly faster, since it doesn't have to bother with type casting. However, I'm assuming that if you provide two identical objects (let's say two strings with the value of "foo"), it still has to check whether the type is the same in both cases : in this one to check whether it's actually the same type or not, and with the equality operator to check whether to cast the object or not.

Thanks for the merge !