Skip to content
This repository has been archived by the owner on Sep 22, 2022. It is now read-only.

Fix code style. #20

Merged
merged 1 commit into from Oct 14, 2018
Merged

Fix code style. #20

merged 1 commit into from Oct 14, 2018

Conversation

bernaferrari
Copy link
Contributor

My eyes hurt when I saw it, so I fixed.

Also a question, wouldn't it be better to use @jvmoverloads on AudioWaveView and replace inflateAttrs with init?

@alxrm
Copy link
Owner

alxrm commented Oct 14, 2018

Now that kind of PR I wanted this whole time! Thanks a lot, good sir, got me laughing 👍

It's actually a common style for functional languages AFAIC, but certainly not for kotlin

Regarding @jvmoverloads, it's nice to have it for secondary constructor, since I'm not accepting primary constructor here, it's just too ugly 🤷‍♀️

@alxrm alxrm merged commit abb0822 into alxrm:master Oct 14, 2018
@alxrm
Copy link
Owner

alxrm commented Oct 14, 2018

And since we're all here, just wanted to say sas to everyone reading this thread

@bernaferrari
Copy link
Contributor Author

If you include the @jvmoverloads you can convert all constructors to primary (https://github.com/bernaferrari/EmojiSlider/blob/d4fae80056c1ccffbaa77f20a56f973cf302d59f/emojislider/src/main/java/com/bernaferrari/emojislider/EmojiSlider.kt#L21), it is a lot better in code readability.

@alxrm
Copy link
Owner

alxrm commented Oct 15, 2018

@bernaferrari Seems like a matter of taste, I don't think it's a lot more readable, thus, as I said before, not going to accept primary constructor

Although I have nothing against @jvmoverloads for the secondary constructor

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

Successfully merging this pull request may close these issues.

None yet

2 participants