Skip to content
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.

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

Change behaviour of .int and .intValue #735

Open
Evertt opened this issue Nov 23, 2016 · 11 comments
Open

Change behaviour of .int and .intValue #735

Evertt opened this issue Nov 23, 2016 · 11 comments
Labels

Comments

@Evertt
Copy link

Evertt commented Nov 23, 2016

As I understand it, SwiftyJSON wants to allow for fuzzy types, because that's how javascript works too, right? I like that, but I don't like how that's implemented.

Right now, .int returns an optional int, which is nil if the value is not a real int. And .intValue provides an int no matter what, which means is tries to convert other values to int values and if that's not possible then it returns a 0.

The thing I don't like about .intValue is that if I get a 0 back, that I don't know if the value wasn't a valid integer, or if the value was in fact a 0. I would like to have a feature that's a bit in between these two features. I would like something that tries to convert the value to an int, and if that's not possible, to return a nil.

Does anyone else like this idea?

@Alex293
Copy link

Alex293 commented Nov 23, 2016

I don't use .intValue only .int and when i need it to be forced I use .int! when you only use those two it pretty clear which one is optional. The thing is unless someone found something better than .optionalInt (.optionalInt! is kinda long an weird no much swifty) for optionals it's not gonna be worth to change.

PS : I'm only a user so take my point considering it :)

@Evertt
Copy link
Author

Evertt commented Nov 23, 2016

.int! is not the same though, because that will only force unwrap an optional. I mean trying to convert a string integer to a real integer. I would like to have that functionality, while still receiving nil if the conversion was not possible.

@ldiqual ldiqual changed the title [discussion] change behaviour of .int and .intValue Change behaviour of .int and .intValue Nov 28, 2016
@lukas2
Copy link
Contributor

lukas2 commented Dec 12, 2016

Why use .intValue? Why don't you simply use if let?

For instance:

if let age = json["age"].int { 
  print(age) 
}

(Or guard let, which often is the cleaner solution.)

@Evertt
Copy link
Author

Evertt commented Dec 14, 2016

Sigh, that's not what I meant. What I mean is that I would want to merge the functionality of .int and .intValue. In other words, I want a computed variable that tries to convert whatever value it holds to an int (like .intValue does), but return a nil when that's not possible (like .int does).

@pedrovereza
Copy link

In other words, I want a computed variable that tries to convert whatever value it holds to an int (like .intValue does), but return a nil when that's not possible (like .int does).

I get what you mean, but what would be the return type of this computed variable? It can't be Int and return nil at the same time:

    public var somethingInt : Int {
        get {
            if (canConvert()) {
                return 2 //An int, as you want
            }
            
            return nil //Compilation error: Nil is incompatible with return type 'Int'
        }
    }

Changing the function to return an Int? is basically what .int already does.

Does that make sense? Did I miss something?

@ThreeTen22
Copy link

ThreeTen22 commented Dec 19, 2016

Since you cant return an Int or nil from the same function you could extend JSON and create your own computed var that returns a tuple with the value from .intValue, and whether or not .int returned nil.
It could look something like:

extension JSON {
   public typealias IntValue = (value:Int, notNil:Bool)
    
   public var intEx : IntValue {
        get {
            //let newSelf = self
            if let intValue = self.int {
                print("inside intvalue")
                return (value: intValue, notNil: true)
            }
            return (value: 0, notNil: false)
        }
    }
    
}

//Ex:
let myInt = myJsonVar["myNumber1"].IntEx  //myJsonVar["myNumber1"] returns 5
let myOtherInt = myJsonVar["myNumber2"].IntEx //myJsonVar["notANumber"] returns "abc"
print(myInt)
if myInt.notNil {
    print(myInt.value)
}
print(myOtherInt)
if myOtherInt.notNil {
    print(myOtherInt.value)
}
//would print:
(5, true)
5
(0, false)

@Evertt
Copy link
Author

Evertt commented Dec 20, 2016

Okay, it proves more difficult to communicate my intention than I had anticipated. I'm going to try one more time to explain and then I'll give up.

Right now .int is implemented basically like this:

public var int: Int? {
    if case .number = self.type {
        return self.rawNumber.intValue
    } else {
        return nil
    }
}

NOTE: This is NOT its real implementation, I'm simplifying things for the purpose of this demonstration.

  • Pro of .int: when the rawValue = "not a number" it will return nil, which clearly conveys that there's no integer.
  • Con of .int: when the rawValue = "2", it will also return nil, while "2" could perfectly well be converted to an integer 2.

And .intValue is basically implemented like this:

public var intValue: Int {
    switch self.type {
    case .string:
        // try to convert string to int.
        // if that's not possible return 0
    case .number:
        // return that number as an int
    case .bool:
        // convert bool to int and return
    default:
        return 0
    }
}

NOTE: This is NOT its real implementation, I'm simplifying things for the purpose of this demonstration.

  • Pro of .intValue: when the rawValue = "2" it will convert it to an integer and return that.
  • Con of .intValue: when the rawValue = "not a number", it will return 0 and then there's no way of knowing if rawValue was "not a number" or if maybe rawValue was actually just the integer 0.

So like I said, I want to merge these two features, like so:

public var newInt: Int? {
    switch self.type {
    case .string:
        // try to convert string to int.
        // if that's not possible return nil
    case .number:
        // return that number as an int
    case .bool:
        // convert bool to int and return
    default:
        return nil
    }
}
  • Pro of .newInt: when the rawValue = "2" it will convert it to an integer and return that.
  • Pro of .newInt: when the rawValue = "not a number" it will return nil, which clearly conveys that there's no usable integer.

Since this new feature is, in my opinion, better than both .int and .intValue, I would argue to make this the new .int and drop the old .int and .intValue.

And this could be applied to bool, and float, double, uint, etcetera.

Please tell me you guys understand what I mean now.

@ThreeTen22
Copy link

ThreeTen22 commented Dec 21, 2016

I get it, it seems like strings that can be converted to numbers do not work with int?, float?, double? etc while the value versions do. It seems it comes from how the var number is calculated compared to numberValue.

If you want to modify the api, you can change the computed var number to this:

public var number: NSNumber? {
   get {
       switch self.type {
            case .Number, .Bool:
            return self.rawNumber

            // added .String case that mimics var numberValue,  but returns nil 
            case .String:
                let decimal = NSDecimalNumber(string: self.object as? String)
                if decimal == NSDecimalNumber.notANumber() {  // indicates parse error
                    return nil
                }
                return decimal
           // end addition

            default:
                return nil
            }
        }
  set {
        self.object = newValue ?? NSNull()
        }
}

The only downside to the above changes is that calling int? on something like "5.25", will return true, and return a rounded number. You could modify int? to check for this.

here is an extension trueInt? that works with the changes above, but will also check if the value is a whole number. If it is not a whole number it will return nil.

extension JSON {
    public var trueInt: Int? {
        get {
            if let num = self.number as? NSDecimalNumber {
                let intLong = num.longValue
                    if num == intLong {
                        return intLong
                    }
                        return nil
            }
            return self.number?.longValue
        }
        set {
            if let newValue = newValue {
                self.object = NSNumber(integer: newValue)
            } else {
                self.object = NSNull()
            }
        }
    }
}

@kevinvanmierlo
Copy link

I would like to see this happen. Here is a pull request which fixes this: #910. For some extra information, Android's core JSON class also does this. If the raw value is a string it tries to convert it to a double and cast it to an int.

@Causaelity
Copy link

@kevinvanmierlo saw your comments and added some more unit tests to make sure that floats and ints also work. Had to add additional code to the Int type so that "3" would generate 3, while "3.14" would generate nil so that it behaves more like Swift typecasting. the intValue version would generate 3 and 0 for the same cases.

@kevinvanmierlo
Copy link

@Causaelity Awesome! Although I wouldn't mind "3.14" generating a "3" (Android's core JSON class does the same). Would love to see your pull request merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants