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 Java enums instead of static int #2

Open
xamino opened this issue Aug 16, 2016 · 3 comments
Open

Use Java enums instead of static int #2

xamino opened this issue Aug 16, 2016 · 3 comments

Comments

@xamino
Copy link

xamino commented Aug 16, 2016

Example of what I mean here.

Instead of using some defined static integers, you could just use a Java enumeration. This would allow code like here to be simplified to a nice switch statement for example. Additionally it will allow type checking at compile time and avoid unwanted double assignment of specific integer values (for example the classical copy & paste error).

Basically nitpicking, but I think it would improve the code readability and maintainability quite a bit.

EDIT: Ups, bad example. If you're using Java 8 though that if-else construct can still be switched out to be a simple switch statement, see here.

EDIT2: In case it is not clear: you could replace many of the static declarations in Constants.java with parameterized enums like shown here.

@JumpMaster
Copy link
Owner

Hi Xamino,

I'm not a professional programmer but am always happy to learn. It seems that creating enums of string isn't straight forward. Strings are required for Intent actions. So I can't change much in Constants.

Also regarding the first example these are used to match BluetoothProfile.java.
http://grepcode.com/file/repo1.maven.org/maven2/org.robolectric/android-all/4.4_r1-robolectric-1/android/bluetooth/BluetoothProfile.java

I started changing to an enum before realising.

I'm using Java 8 so switch statements make sense. First few changed as you can see here.
https://github.com/JumpMaster/WheelLogAndroid/blob/master/app/src/main/java/com/cooper/wheellog/MainActivity.java#L126

Any other best practices are welcome!

@xamino
Copy link
Author

xamino commented Aug 17, 2016

Always glad to be of assistance!

Strings are required for Intent actions.

these are used to match BluetoothProfile.java

Ah yes, bluetooth. Shudder Okay – as I stated I was nitpicking anyway. 😁

Wheel type also seems to be a good candidate to be "enumified", see here.

This is more of an architecture issue, so you might not want to change this, but I'll put it out there anyway. It may be nice to split KingSong and Gotway decoding logic into separate classes with a shared interface in a sub package. That way it would be trivial to add support for new wheels by simply adding a new class which implements the shared interface. Additionally it would move all wheel specific logic into clearly separated logical units and make the app more "general".

@JumpMaster
Copy link
Owner

I've changed WheelType to an enum. I like the interface idea. Will have to do some research to make sure it's implemented correctly. It's not something I've done before.

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

No branches or pull requests

2 participants