-
Notifications
You must be signed in to change notification settings - Fork 134
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
Refactoring StorageHelper class #851
Conversation
mSQLiteOpenHelper = helper; | ||
String[] getColumnNames() { | ||
|
||
// TODO: Below line doesn't look efficient to get column names. Could use "PRAGMA table_info(table-name)" to avoid getting all data back from database just to get column names. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about let's do this TODO now? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is used only in unit tests we could just remove the comment
persistence.close(); | ||
} | ||
|
||
/* There is a error log. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a
=> an
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix all occurrence in this file
Constants.loadFromContext(sContext); | ||
} | ||
|
||
private static int getCursorSize(Cursor cursor) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can use cursor.getCount() now that we removed a layer.
long databaseId = mDatabaseStorage.put(contentValues); | ||
long databaseId = mDatabaseManager.put(contentValues); | ||
if (databaseId == -1) { | ||
AppCenterLog.warn(LOG_TAG, "Failed to store a log to the Persistence database for log type " + log.getType() + "."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to throw PersistenceException
try { | ||
Cursor cursor = mDatabaseManager.getCursor(builder, new String[]{group}, true); | ||
count = cursor.getCount(); | ||
cursor.close(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try finally
String[] selectionArgs = new String[]{value.toString()}; | ||
Cursor cursor = getCursor(builder, selectionArgs, false); | ||
ContentValues values = cursor.moveToFirst() ? buildValues(cursor) : null; | ||
cursor.close(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
finally
mSQLiteOpenHelper = helper; | ||
String[] getColumnNames() { | ||
|
||
// TODO: Below line doesn't look efficient to get column names. Could use "PRAGMA table_info(table-name)" to avoid getting all data back from database just to get column names. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is used only in unit tests we could just remove the comment
|
||
/* groupCount should be <= 9. */ | ||
final int groupCount = 4; | ||
final int groupCount = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this change?
HasCHANGELOG.md
been updated?