patternjavaMajor
Android Flashlight app
Viewed 0 times
flashlightandroidapp
Problem
This is my first project, a Flashlight application for Android-based devices:
```
public class main extends Activity
{
private static final byte MENU_EXIT = 0, MENU_ABOUT = 1;
private static boolean FlashlightState;
private static long BackPressed;
private static ImageView Lamp;
private Flashlight FLASHLIGHT = null;
@Override
public void onCreate(Bundle savedInstanceState)
{
super.onCreate(savedInstanceState);
super.setContentView(R.layout.main);
super.getWindow().addFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN);
FlashlightState = false;
Lamp = (ImageView) findViewById(R.id.imgVw_Lamp);
FLASHLIGHT = new Flashlight(this.getApplicationContext());
if (Flashlight.hasFlashlight) FLASHLIGHT.Open();
Lamp.setOnClickListener(new OnClickListener()
{
@Override
public void onClick(View arg0)
{
Lamp.setEnabled(false);
if (getSystemService(VIBRATOR_SERVICE) != null)
{
AudioManager VibratorState = (AudioManager) getApplicationContext().getSystemService(Context.AUDIO_SERVICE);
if (VibratorState.getRingerMode() != AudioManager.RINGER_MODE_SILENT)
{
Vibrator Switch = (Vibrator) getSystemService(Context.VIBRATOR_SERVICE);
Switch.vibrate(50);
}
VibratorState = null;
}
if (!FlashlightState) turnOnFlashlight();
else turnOffFlashlight();
Lamp.setEnabled(true);
return;
}
});
return;
}
@Override
protected void onStart()
{
super.onStart();
BackPressed = 0;
if (Flashlight.hasFlashlight) FLASHLIGHT.Open();
return;
}
@Override
protected void onPause()
{
super.onPause();
```
public class main extends Activity
{
private static final byte MENU_EXIT = 0, MENU_ABOUT = 1;
private static boolean FlashlightState;
private static long BackPressed;
private static ImageView Lamp;
private Flashlight FLASHLIGHT = null;
@Override
public void onCreate(Bundle savedInstanceState)
{
super.onCreate(savedInstanceState);
super.setContentView(R.layout.main);
super.getWindow().addFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN);
FlashlightState = false;
Lamp = (ImageView) findViewById(R.id.imgVw_Lamp);
FLASHLIGHT = new Flashlight(this.getApplicationContext());
if (Flashlight.hasFlashlight) FLASHLIGHT.Open();
Lamp.setOnClickListener(new OnClickListener()
{
@Override
public void onClick(View arg0)
{
Lamp.setEnabled(false);
if (getSystemService(VIBRATOR_SERVICE) != null)
{
AudioManager VibratorState = (AudioManager) getApplicationContext().getSystemService(Context.AUDIO_SERVICE);
if (VibratorState.getRingerMode() != AudioManager.RINGER_MODE_SILENT)
{
Vibrator Switch = (Vibrator) getSystemService(Context.VIBRATOR_SERVICE);
Switch.vibrate(50);
}
VibratorState = null;
}
if (!FlashlightState) turnOnFlashlight();
else turnOffFlashlight();
Lamp.setEnabled(true);
return;
}
});
return;
}
@Override
protected void onStart()
{
super.onStart();
BackPressed = 0;
if (Flashlight.hasFlashlight) FLASHLIGHT.Open();
return;
}
@Override
protected void onPause()
{
super.onPause();
Solution
Use camelcase for your variable names.
Instead of:
(This is to increase readability so you can tell the difference between class and object more clearly)
Use braces for if else statements
In your code you use:
More appropriate would be to use:
If you absolutely want to omit braces, omit them for single-statement solely-if clauses only. Such as:
Using killProcess is a bad habit. (Destroy all your activities instead)
If you want to quit an App, just make sure all activities in the app are
If you want to quit an activity, use
Redundant return statements at the end of void functions.
Most of your functions have the statement
Redundant final keyword in private method declarations
Your private methods are not going to be overridden or anything, they don't need the final keyword.
Optional:
Refactor away your turnOn and turnOff functions.
You currently call two different functions that do a roughly similar thing depending on whether you toggle the flashlight on or off.
Instead of the following:
You could do something like:
In toggle flashlight you could do:
There's a dozen other ways to design it. (Where do you want to do the actual toggling of your boolean, etc. But this is just one example)
Instead of:
public boolean MyBoolean write public boolean myBoolean(This is to increase readability so you can tell the difference between class and object more clearly)
Use braces for if else statements
In your code you use:
if (!FlashlightState) turnOnFlashlight();
else turnOffFlashlight();More appropriate would be to use:
if(!FlashlightState) {
turnOnFlashlight();
} else {
turnOffFlashlight();
}If you absolutely want to omit braces, omit them for single-statement solely-if clauses only. Such as:
if(bool) doSomething(); //No else statementsUsing killProcess is a bad habit. (Destroy all your activities instead)
If you want to quit an App, just make sure all activities in the app are
finish();'ed.If you want to quit an activity, use
finish();, this will call onDestroy() on your activity. You should let the OS handle the lifecycle of your activities as much as possible. Assuming that you have a single activity app, calling finish(); should destroy the only activity in your app, thus quitting it at the first opportunity.Redundant return statements at the end of void functions.
Most of your functions have the statement
return; in them at the end. This is completely redundant and serves no use.Redundant final keyword in private method declarations
Your private methods are not going to be overridden or anything, they don't need the final keyword.
private: This method cannot be overridden by a subclassfinal: This method cannot be overridden by a subclassprivate final: This method cannot be overridden by a subclass and this method cannot be overridden by a subclass (Redundant)Optional:
Refactor away your turnOn and turnOff functions.
You currently call two different functions that do a roughly similar thing depending on whether you toggle the flashlight on or off.
Instead of the following:
if (!FlashlightState) turnOnFlashlight();
else turnOffFlashlight();You could do something like:
toggleFlashLight();In toggle flashlight you could do:
private void toggleFlashLight() {
FlashlightState = !FlashlightState;
if(FlashlightState) {
//Turn it on;
} else {
//Turn it off;
}
}There's a dozen other ways to design it. (Where do you want to do the actual toggling of your boolean, etc. But this is just one example)
Code Snippets
if (!FlashlightState) turnOnFlashlight();
else turnOffFlashlight();if(!FlashlightState) {
turnOnFlashlight();
} else {
turnOffFlashlight();
}if (!FlashlightState) turnOnFlashlight();
else turnOffFlashlight();toggleFlashLight();private void toggleFlashLight() {
FlashlightState = !FlashlightState;
if(FlashlightState) {
//Turn it on;
} else {
//Turn it off;
}
}Context
StackExchange Code Review Q#63012, answer score: 26
Revisions (0)
No revisions yet.