Feature/failable uicolor init #273

danielavrammindera · 2025-06-17T14:12:37Z

Checklist

I've rebased my changes on top of master
I've built and run the project to see all new and existing tests pass
I've followed the Mindera swift style guide
I've read the Contribution Guidelines
I've updated the documentation if necessary.

Motivation and Context

Would be helpful to have a failable init for UIColor. In the project i'm working on we're using Alicerce and would be nice to have this functionality readily available from Alicerce

Description

The changes simply add a new convenience init for UIColor, while still keeping the old one (same params) that used to fatalError on fail

p4checo

Nice addition, although this helper has historically been used mostly from static (hex color) values, so we are "certain" they are valid ar runtime.

Nonetheless, it makes sense to make it a bit more robust with proper error handling.

p4checo · 2025-06-17T14:32:51Z

Sources/Persistence/DiskMemoryPersistenceStack.swift

 }

-private final class DiskMemoryBlockOperation: BlockOperation {
+private final class DiskMemoryBlockOperation: BlockOperation, @unchecked Sendable {


please remove concurrency changes as we haven't migrated the project to using Swift Concurrency yet

i reverted this but now on my machine the project doesn't compile. is it because Xcode 16 or smth?

yes, it's probably because you're building it in Swift 6 mode or swft 5.10 with full concurrency checking

Sources/Extensions/UIKit/UIColor.swift

p4checo · 2025-07-28T16:24:05Z

Sources/Extensions/UIKit/UIColor.swift

    }

-    var hexString: String {
+    convenience init(hex: String) {


if we're making the initializer failable, we probably shouldn't keep the non-failable one. Users should probably migrate to proper try/catch, fallback to try? or force-unwrap if they are certain things are sound (as should be the case for existing codebases).

All this functionality would probably be better expressed as a macro that would convert the string to a UIColor, similar to the #URL() macro here. This hex to color conversion is mostly done statically rather than dynamically, anyway, right?

not sure tbh. there was an ask on the KF project to change the color of a button dynamically, based on whatever Firebase value we had somewhere. so in that case we'd need a dynamic UIColor constructor.

I didn't want to break backwards compatibility, which is why i thought it safe to keep the old init (which fatalErrors) and add the new failable one, should any project using Alicerce need dynamic UIColor initialization (currently KF doesn't need it anymore, but it did when i made the MR)

if it were up to me i'd keep the old just to not break compatibility, but happy to change if we feel like going in that direction

off topic: did some cleanup (at least i'd like to think so) in the last commit. can revert if too much / irrelevant. lemme know

If this is no longer a need for your current project, is it still worth adding?

Most use cases that I am aware rely on static values and not dynamic ones, and adding this new capability without an actual need makes me question the effort and impact on API.

Furthermore, nowadays there are more modern/cleaner ways to do this, with color assets (that accept hex value), native regexes to validate values if needed, and even macros that could create and validate color values at compile time.

You added some nice improvements though, so thanks for that! 🙏🏼

What are your thoughts?

If you still think it's worth adding, then I think we also need to add coverage on the new functionality 🤓

depending on which hour of the day you ask me, i'll answer either "yes" or "no" 😅

Why yes

still has same functionality of fatalError'ing init

makes the logic a tiny bit cleaner / readable

adds new functionality (failable init) which goes very much hand in hand with the initially implemented init. (imo it should have been failable from the start, not fatalError'ing)

Why no

newly added functionality is no longer required by any consumer; now exists just for a "what if we need it in the future" scenario

UIColor.swift is not longer; before these changes, even if there was a bit of code duplication you could see at a first glance all the file. now it requires a bit more reading time

i added some Unit Tests for the new functionality. let me know if I:

should add anything extra

should discard the PR

i'm fine with either, as i'm a bit in between myself whether these changes are still relevant or not

Thanks for the sanity check. I am torn myself as well 😅

All things considerad, I think that since we already have this new functionality essentially done, as long as we don't make this a breaking change (i.e. we keep the existing fatalError non throws API), then I think we can merge it.

We do have some issues with CI, namely on the CocoaPods validation, which need to be sorted before we can merge this though 🙈

apologies for late reply. got sidetracked with some project work. i didn't manage to fix the MR. looks like some sort of a fastlane problem? can't really tell. i'll be on holiday starting 11th Aug until 24th Aug. Can pick this one up when i return if needed. If you change your mind however, feel free to close it, i don't mind 😁

Sources/Extensions/UIKit/UIColor.swift

This reverts commit b1eab85.

codecov · 2025-07-28T17:06:16Z

Codecov Report

❌ Patch coverage is 97.36842% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.10%. Comparing base (1ee2f11) to head (aafd2b8).

Files with missing lines	Patch %	Lines
Sources/Extensions/UIKit/UIColor.swift	97.36%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #273      +/-   ##
==========================================
+ Coverage   95.18%   96.10%   +0.92%     
==========================================
  Files         101      101              
  Lines        3445     5106    +1661     
==========================================
+ Hits         3279     4907    +1628     
- Misses        166      199      +33

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sources/Extensions/Foundation/String.swift

p4checo · 2025-07-29T19:08:23Z

Sources/Persistence/DiskMemoryPersistenceStack.swift

 }

-private final class DiskMemoryBlockOperation: BlockOperation {
+private final class DiskMemoryBlockOperation: BlockOperation, @unchecked Sendable {


yes, it's probably because you're building it in Swift 6 mode or swft 5.10 with full concurrency checking

p4checo · 2025-07-29T19:12:48Z

Sources/Extensions/UIKit/UIColor.swift

    }

-    var hexString: String {
+    convenience init(hex: String) {


If this is no longer a need for your current project, is it still worth adding?

Most use cases that I am aware rely on static values and not dynamic ones, and adding this new capability without an actual need makes me question the effort and impact on API.

Furthermore, nowadays there are more modern/cleaner ways to do this, with color assets (that accept hex value), native regexes to validate values if needed, and even macros that could create and validate color values at compile time.

You added some nice improvements though, so thanks for that! 🙏🏼

What are your thoughts?

If you still think it's worth adding, then I think we also need to add coverage on the new functionality 🤓

Daniel Avram added 3 commits June 17, 2025 17:03

add fail-able UIColor init

3f5d251

fix compilation

b1eab85

fix localizedDescription

73f4cfd

danielavrammindera self-assigned this Jun 17, 2025

p4checo requested changes Jul 28, 2025

View reviewed changes

Daniel Avram added 2 commits July 28, 2025 19:56

Revert "fix compilation"

5d61c9a

This reverts commit b1eab85.

mr comments

3994cba

cleanup

0c92064

p4checo requested changes Jul 29, 2025

View reviewed changes

mr comments + add some unit tests

aafd2b8

Feature/failable uicolor init #273

Are you sure you want to change the base?

Feature/failable uicolor init #273

Uh oh!

Conversation

danielavrammindera commented Jun 17, 2025

Checklist

Motivation and Context

Description

Uh oh!

p4checo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Why yes

Why no

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jul 28, 2025 •

edited

Loading