RM14476 - Lab level 3. Services #19
Conversation
|
There is no need in showing notification when calculation canceled. Just dismiss Ongoing Notification. |
|
It shall not be possible to swipe out Ongoing Notification with calculation progress. |
|
Let's place precision selector and Start/Cancel button in center of screen |
…' into RM_14476_Lab_level_3_Services
| return complete.digest(); | ||
| } | ||
|
|
||
| public static String getMD5Checksum(String filename) throws Exception { |
There was a problem hiding this comment.
Public methods have to be placed before private in class.
Also not a best name for method. It would be better to rename it to calculateMD5ChecksumForFile
There was a problem hiding this comment.
Also I think that there is no need in two methods - all code can be put in single method with appropriate comments where required
| import java.io.InputStream; | ||
| import java.security.MessageDigest; | ||
|
|
||
| public class MD5CheckSum { |
There was a problem hiding this comment.
Such kind of classes with only static helper methods is appropriate to name with postfix ...Utils.
For this class it would be MD5ChecksumUtils or ChecksumUtils
…3_Services Conflicts: CustomView/.classpath SMSResponder/.classpath SMSResponder/res/values/styles.xml
|
|
||
| public class MD5CheckSum { | ||
|
|
||
| private static byte[] createChecksum(String filename) throws Exception { |
There was a problem hiding this comment.
It's bad idea in most cases to declare that method just throws Exception
In fact it have to be little bit more detailed throws NoSuchAlgorithmException, IOException
or even throws NoSuchAlgorithmException, FileNotFoundException, IOException
This way code that calling this method will have possibility to handle each type of exception appropriately.
|
After first install on device when I'm trying to start calculations app crashes with next stack trace |
| import android.widget.Spinner; | ||
| import android.widget.TextView; | ||
|
|
||
| public class MainActivity extends Activity { |
There was a problem hiding this comment.
In this file there are unused imports which should be deleted (try to use ctrl+O shortcut =)
|
All IO operations have to be done asynchronously outside of main thread. |
|
Algorithm is still slow. It's about 10 times slower then Super Pi |
| Intent intent = new Intent(this, PiCalculateService.class); | ||
| intent.putExtra(PiCalculateService.PRECISION_KEY, precisions_.getSelectedItem().toString()); | ||
| startService(intent); | ||
| File dir = new File(Environment.getExternalStorageDirectory().toString() + "/cash_pi"); |
There was a problem hiding this comment.
It's better to store all calculated PI values in Context.getExternalFilesDir("calculated_pi_values"); This folder will be deleted on app uninstall. Read more about it in JavaDocs
There was a problem hiding this comment.
=) Be careful with typos. Cash means money. You've probably meant cache.
|
Please, extract all PI calculation logic into separate class. Also mention algorithm which you're using to calculate PI and link to where you've found it. |
| result = String.copyValueOf(charString); | ||
| } | ||
| } catch (FileNotFoundException e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
Do proper Exception handling here. At least show toast to user with appropriate error message.
|
Try to use enum for listing all Precisions. In Java enum is fully functional object |
|
Hash sum check for calculated PI value have to be part of service work, not MainActivity. |
pi calculate in NDK library;
RM_14476_Lab_level_3_Services
time is measured in milliseconds.
http://redmine.ruswizards.com:8100/redmine/issues/14476