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

Move to generic types #17

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

Move to generic types #17

wants to merge 3 commits into from

Conversation

liuxuan30
Copy link
Collaborator

@liuxuan30 liuxuan30 commented Jan 16, 2018

I'm actually trying to use ChartsRealm. However when doing so, like in a realm query to SomeObject, the API cannot take Results<SomeObject>, as it's not possible to convert to Results<Object>, so I think we should use generic types here.

WIP

I found there is a crash realm/realm-swift#5555, so cannot test the whole workflow. or something still wrong.

fix a dead loop in @objc public convenience init(results: RLMResults<RLMObject>?, yValueField: String)
@jekirl
Copy link

jekirl commented Jan 17, 2018

👍 👍
Was running into the same issue....would be very nice to have generic types working. I ran into the same issue as you did in realm/realm-swift#5555 in my attempt to implement it.

@liuxuan30
Copy link
Collaborator Author

liuxuan30 commented Jan 18, 2018

From the discuss, RLMObject and Object both inherit from RLMObjectBase, so to actually read data, we need to provide an interface for both of them.

It would take more time, as it's almost a rewrite of this repo.

using RLMObjectBase as base type, and later cast to RLMObject or Object
_results = results

_results = results as? RLMResults<RLMObjectBase>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jjatie Not sure if you can see this. Need your help here

I learned that Results<SubObject> cannot be converted to Results<Object>, per https://stackoverflow.com/questions/48125923/realmswift-cannot-cast-resultssomeojbect-to-resultsobject/48126909?noredirect=1#comment83231956_48126909

However, here I can cast RLMResults<RLMObject> to results as? RLMResults<RLMObjectBase>, without problem.

I am kind of confused here. Any idea? Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll take a look this week.

Copy link
Collaborator Author

@liuxuan30 liuxuan30 Jan 20, 2018

Choose a reason for hiding this comment

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

further experiments show that, I could even pass a Result<SomeObejct> into converted = ObjectiveCSupport.convert(object: results!) as? RLMResults<RLMObject>, and it can even convert Result<SomeObejct> to RLMResults<RLMObject>, even SomeObejct and RLMObject has no relationship.

However, at run time it's really SomeObejct inside the RLMResults, not RLMObject type. That's why my PR works

I asked this in realm/realm-swift#5555, to see if they add any.

{
let value = object[_yValueField!]
var value: Any?
if let object = object as? RLMObject
Copy link
Collaborator Author

@liuxuan30 liuxuan30 Jan 18, 2018

Choose a reason for hiding this comment

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

@jjatie Do you have better solution that we could reduce the duplicated code like this pattern.

Basically, we take RLMObjectBase, but later need to cast it to a concrete class type to get the value. Objc is using RLMObject, while Swift is using Object.

I don't come up a better one here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

first, you should use let value: Object. It doesn't need to be optional or a variable because you know you're going to initialize it right away.

Second, you can write it like this

let value = (object as? RLMObject)[_yValueField!] ?? (object as! Object)[_yValueField!]

or

let object = (object as? RLMObject) ?? (object as! Object)
let value = object[_yValueField!]

Copy link
Collaborator Author

@liuxuan30 liuxuan30 Jan 21, 2018

Choose a reason for hiding this comment

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

seems neither of them works.
first statement

let value = (object as? RLMObject)[_yValueField!] ?? (object as! Object)[_yValueField!]

gives me Type 'RLMObject?' has no subscript members, as the left operand is optional, while the right operand would work.

second statement gives me Type 'RLMObjectBase' has no subscript members

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oops, should have been
let value = (object as? RLMObject)?[_yValueField!] ?? (object as! Object)[_yValueField!]

@jjatie
Copy link
Collaborator

jjatie commented Jan 18, 2018

@liuxuan30 I think this is a good start. I think this would be good to release alongside Charts 4.0.0. I will start to get familiar with the ChartsRealm repo so I can help you with this.

@liuxuan30
Copy link
Collaborator Author

liuxuan30 commented Jan 21, 2018

I was actually trying to use Realm in my app with Charts but in reality it barely works :(

Take your time, as this is a low priority. At least we have something would work..

I feel the sole purpose of this repo is we don't have to ask user to re-setup the data again, as Realm has auto-update feature, we only need to call data.notifyDataChanged and chartView.notifyDataSetChanged so it will redraw with latest results.

If converting Results<Object> to RLMResult<RLMObject> like this PR is a valid practice, we could move forward then, as I don't see other low cost solution.

I would admit now supporting both Objc and Swift is a headache like this..

@liuxuan30
Copy link
Collaborator Author

I just leave a comment here for later reference and discuss.

I'm not sure if we should change
ObjectiveCSupport.convert(object: results!) as? RLMResults<RLMObject> to
ObjectiveCSupport.convert(object: results!) as? RLMResults<RLMObjectBase> here.

Either of the cast would "work" fine, but in terms of semantic, not sure if we should use their common parent class RLMObjectBase, or just use the Objc counterpart RLMObject

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

Successfully merging this pull request may close these issues.

3 participants