Skip to content

Feature/failable uicolor init #273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Feature/failable uicolor init #273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3f5d251
add fail-able UIColor init
Jun 17, 2025
b1eab85
fix compilation
Jun 17, 2025
73f4cfd
fix localizedDescription
Jun 17, 2025
5d61c9a
Revert "fix compilation"
Jul 28, 2025
3994cba
mr comments
Jul 28, 2025
0c92064
cleanup
Jul 28, 2025
aafd2b8
mr comments + add some unit tests
Jul 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Sources/Extensions/Foundation/String.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ public extension String {
func substring(with nsRange: NSRange) -> String {
return nsString.substring(with: nsRange) as String
}

func prepending(_ other: String) -> String {
return other + self
}
}

public extension String {
Expand Down
104 changes: 59 additions & 45 deletions Sources/Extensions/UIKit/UIColor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,69 +1,63 @@
import UIKit

public extension UIColor {
private static let divisor = CGFloat(255)

private typealias Components = (red: CGFloat, green: CGFloat, blue: CGFloat, alpha: CGFloat)

convenience init(hex: String) {
let hex = hex.trimmingCharacters(in: .whitespacesAndNewlines)
.replacingOccurrences(of: "#", with: "")
convenience init(hexValue: String) throws {
let hex = hexValue
.filter { $0.isLetter || $0.isNumber }
.prepending("ff")
.suffix(8)
.asString

guard hex.count == 8 else {
throw ColorError.invalidHexSize(hexValue)
}

var hexValue: UInt64 = 0
var hexInt: UInt64 = 0

guard Scanner(string: hex).scanHexInt64(&hexValue) else {
fatalError("😱 Cannot convert string into `UInt64`")
guard Scanner(string: hex).scanHexInt64(&hexInt) else {
throw ColorError.invalidHexValue(hexValue)
}

let components: Components = {
switch hex.count {
case 6: return UIColor.components(fromHex6: hexValue)
case 8: return UIColor.components(fromHex8: hexValue)
default: fatalError("😱 hex size not supported 😇")
}
}()
let components = UIColor.components(hex: hexInt)

self.init(red: components.red, green: components.green, blue: components.blue, alpha: components.alpha)
}

var hexString: String {

var components = UIColor.components(fromHex6: 0)
getRed(&components.red, green: &components.green, blue: &components.blue, alpha: &components.alpha)

let r = Int(components.red * UIColor.divisor)
let g = Int(components.green * UIColor.divisor)
let b = Int(components.blue * UIColor.divisor)
let rgb: Int = r << 16 | g << 8 | b << 0
convenience init(hex: String) {
Copy link
Collaborator

@p4checo p4checo Jul 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

@danielavrammindera danielavrammindera Jul 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

@danielavrammindera danielavrammindera Jul 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

@p4checo p4checo Jul 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🤓

Copy link
Author

@danielavrammindera danielavrammindera Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Why yes

  1. still has same functionality of fatalError'ing init
  2. makes the logic a tiny bit cleaner / readable
  3. 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

  1. newly added functionality is no longer required by any consumer; now exists just for a "what if we need it in the future" scenario
  2. 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:

  1. should add anything extra
  2. 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

Copy link
Collaborator

@p4checo p4checo Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🙈

Copy link
Author

@danielavrammindera danielavrammindera Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😁

do {
try self.init(hexValue: hex)
} catch {
fatalError(error.localizedDescription)
}
}

return String(format:"#%06x", rgb)
var hexString: String {
String(format:"#%06x", (asHexIntWithAlpha & 0x00FFFFFF))
}

var hexStringWithAlpha: String {

var components = UIColor.components(fromHex8: 0)
getRed(&components.red, green: &components.green, blue: &components.blue, alpha: &components.alpha)

let a = Int(components.alpha * UIColor.divisor)
let r = Int(components.red * UIColor.divisor)
let g = Int(components.green * UIColor.divisor)
let b = Int(components.blue * UIColor.divisor)
let argb: Int = a << 24 | r << 16 | g << 8 | b << 0

return String(format:"#%08x", argb)
String(format:"#%08x", asHexIntWithAlpha)
}
}

// MARK: - Private Methods
private extension UIColor {
typealias Components = (red: CGFloat, green: CGFloat, blue: CGFloat, alpha: CGFloat)

private static func components(fromHex6 hex: UInt64) -> Components {
let red = CGFloat((hex & 0xFF0000) >> 16) / UIColor.divisor
let green = CGFloat((hex & 0x00FF00) >> 8) / UIColor.divisor
let blue = CGFloat(hex & 0x0000FF) / UIColor.divisor
enum ColorError: LocalizedError {
case invalidHexValue(String)
case invalidHexSize(String)

return (red, green, blue, 1.0)
var localizedDescription: String {
switch self {
case let .invalidHexValue(hex): "😱 Cannot convert `#\(hex)` into `UInt64`"
case let .invalidHexSize(hex): "😱 Hex size of `#\(hex)` not supported 😇"
}
}
}

private static func components(fromHex8 hex: UInt64) -> Components {
static let divisor = CGFloat(255)

static func components(hex: UInt64) -> Components {

let alpha = CGFloat((hex & 0xFF000000) >> 24) / UIColor.divisor
let red = CGFloat((hex & 0x00FF0000) >> 16) / UIColor.divisor
Expand All @@ -72,4 +66,24 @@ public extension UIColor {

return (red, green, blue, alpha)
}

var asHexIntWithAlpha: Int {

var components = UIColor.components(hex: 0)
getRed(&components.red, green: &components.green, blue: &components.blue, alpha: &components.alpha)

let a = Int(components.alpha * UIColor.divisor)
let r = Int(components.red * UIColor.divisor)
let g = Int(components.green * UIColor.divisor)
let b = Int(components.blue * UIColor.divisor)
let argb: Int = a << 24 | r << 16 | g << 8 | b << 0

return argb
}
}

private extension String.SubSequence {
var asString: String {
String(self)
}
}
21 changes: 21 additions & 0 deletions Tests/AlicerceTests/Extensions/UIKit/UIColorTestCase.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,25 @@ final class UIColorTestCase: XCTestCase {
XCTAssertEqual(ciTransparentColor.blue, 1.0)
XCTAssertEqual(ciTransparentColor.alpha, 0.0)
}

func testGibberishInput_WithValidColorHex_ShouldSucceed() throws {
let gibberish = "@ff&ff*ff%f#f"

let color = try UIColor(hexValue: gibberish)

let ciColor = CIColor(color: color)

XCTAssertEqual(ciColor.red, 1.0)
XCTAssertEqual(ciColor.green, 1.0)
XCTAssertEqual(ciColor.blue, 1.0)
XCTAssertEqual(ciColor.alpha, 1.0)
}

func testGibberishInput_WithInvalidColorHex_ShouldFail() {
let string = "random string"

let color = try? UIColor(hexValue: string)

XCTAssertNil(color)
}
}
7 changes: 7 additions & 0 deletions Tests/HostAppRequiringTests/Extensions/StringTestCase.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,11 @@ class StringTestCase_Localizable: XCTestCase {

XCTAssertNotEqual(localizedHelperTestWithArguments, resultString)
}

func testPrepending_ShouldSucceed() {
let world = "world!"
let hello = "Hello, "

XCTAssertEqual(world.prepending(hello), "Hello, world!")
}
}
Loading