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

Use generics instead of unchecked casts for more type-safety #47

Closed
wants to merge 4 commits into from
Closed

Use generics instead of unchecked casts for more type-safety #47

wants to merge 4 commits into from

Conversation

austynmahoney
Copy link

I've changed the library (and the example app) to use generics to enforce type-safety at compile time. This keeps you from having to cast every Object that comes back from the view. The app code becomes much cleaner at the expense of generic use and some object casting in the library.

Closes #46

@austynmahoney
Copy link
Author

This issue with using TokenCompleteTextView<Cursor> is the Cursor lifespan. We can't really keep those around in selectedObject and the like.

I'd like to explore adding some kind of conversion method to assist in translating Cursor to T, so we don't keep the Cursor returned from the adapter around longer than it needs to be. Any suggestions on how to implement this? A sample app implementing a CursorAdapter and this view would be great to play around with also.

@ross
Copy link
Contributor

ross commented Mar 27, 2014

i'll see if i can throw an example app together this weekend, but it'll be a pretty busy one for me so no promises.

@ross
Copy link
Contributor

ross commented Mar 31, 2014

just pushed up a (very) quick and dirty cursor backed example to a branch on my fork, https://github.com/ross/TokenAutoComplete/tree/cursor_ex

should be familiar (it's just a quick swap on the existing example) and enough to see what's involved.

originally wanted to leave both exaples intact, but i didn't have time to do so. there's a lot of stuff that could be cleaner regardless, some of which i think you're on track to address. let me know if you have any questions.

@mgod
Copy link
Contributor

mgod commented Jun 2, 2014

@ross @austynmahoney I've finally had some time to look at this in more detail. @ross it seems like @austynmahoney's code will still work with Cursors by simply using the type TokenCompleteTextView<Object> and doing the same conversions you're currently doing. If that sounds right to you two, I'd like to pull this change and open a new issue for improving adapter support.

@austynmahoney
Copy link
Author

Sounds good with me. I've moved onto other projects that do not use this for now, but let me know if you need some specific help with the adapter stuff.

@ross
Copy link
Contributor

ross commented Jun 10, 2014

i guess it's not the end of the world to do Object so i guess that'll work. i haven't been working on the project i was using this in for a while now and at at least for now it's working fine with the older code.

@Sirrah
Copy link
Contributor

Sirrah commented Apr 8, 2015

@mgod Are you still interested in merging this?

@mgod
Copy link
Contributor

mgod commented Apr 8, 2015

@Sirrah I've spent some time looking to make this change in the core app and keep running into issues with getting it working where I think I may just not know enough about Java to make it happy. I probably won't merge this particular pull request as the library has changed significantly since then, but this is something I would like to do.

@Sirrah
Copy link
Contributor

Sirrah commented Apr 8, 2015

Do you have a more current branch you've been working on or can you expand on the issues you ran into? I'll look into adding the generics myself as well.

@mgod
Copy link
Contributor

mgod commented Apr 8, 2015

Actually, looking over this PR again, I can't remember what I had trouble with. It seems like this should be pretty straightforward? At the time I was fussing with this, TokenImageSpan may have been a static class on my local copy which might have been the issue.

@Sirrah
Copy link
Contributor

Sirrah commented Apr 8, 2015

I just rebased this pr to the current master and gave it a more thorough look. It seems fine so far. I'll have to make some changes to my current project to test it out (currently I'm using 1.2.1 for the compatibility with older Android APIs). I'll come back to this tomorrow.

@Sirrah
Copy link
Contributor

Sirrah commented Apr 9, 2015

I've created a new pull request #120 updated for the current master. I did run into an issue with TokenImageSpan but was able to resolve it with an explicit cast.

@mgod
Copy link
Contributor

mgod commented Apr 22, 2015

Merged #120, which was an updated version of this with all of it's changes.

@mgod mgod closed this Apr 22, 2015
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.

Use generics instead of unchecked casts for more type-safety
5 participants