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

Possible Code refactorings #2153

Open
35 of 48 tasks
theScrabi opened this issue Mar 16, 2018 · 3 comments
Open
35 of 48 tasks

Possible Code refactorings #2153

theScrabi opened this issue Mar 16, 2018 · 3 comments
Assignees
Labels

Comments

@theScrabi
Copy link
Contributor

theScrabi commented Mar 16, 2018

I've worked on the android app code for a little while now, and found some code which could be refactored or even removed. Here is my list:

Things to DELETE

Classes not in use anymore

  • public class NavigationDrawerItem {

  • public class ChangelogDialog extends DialogFragment {

  • public class UploadSourceDialogFragment extends DialogFragment {

  • public class SslValidatorDialog extends Dialog {

  • public static class UploadRequester {
    }

  • public interface OnUploadCompletedListener extends Runnable {

  • This class seems important, however it's not in use:

    public abstract class RecursiveFolderObserver extends FileObserver {

  • import java.util.ArrayList;
    import java.util.List;
    public class GroupAdapter {
    public String string;
    public final List<String> children = new ArrayList<String>();
    public GroupAdapter(String string) {
    this.string = string;
    }
    }

  • import java.io.File;
    import com.owncloud.android.R;
    import android.content.Context;
    import android.content.Intent;
    import android.net.Uri;
    import android.os.Environment;
    import android.view.LayoutInflater;
    import android.view.View;
    import android.view.View.OnClickListener;
    import android.view.ViewGroup;
    import android.widget.ArrayAdapter;
    import android.widget.TextView;
    public class LogListAdapter extends ArrayAdapter<String> {
    private Context context = null;
    private String[] values;
    private Uri fileUri = null;
    public LogListAdapter(Context context, String[] values) {
    super(context, R.layout.log_item, values);
    this.context = context;
    this.values = values;
    }
    @Override
    public View getView(final int position, View convertView, ViewGroup parent) {
    LayoutInflater inflater = (LayoutInflater) context
    .getSystemService(Context.LAYOUT_INFLATER_SERVICE);
    View rowView = inflater.inflate(R.layout.log_item, parent, false);
    TextView listText = (TextView) rowView.findViewById(R.id.log_item_single);
    listText.setText(values[position]);
    listText.setTextSize(15);
    fileUri = Uri.fromFile(new File(Environment.getExternalStorageDirectory()+File.separator+"owncloud"+File.separator+"log"+File.separator+values[position]));
    listText.setOnClickListener(new OnClickListener() {
    @Override
    public void onClick(View v) {
    Intent emailIntent = new Intent(android.content.Intent.ACTION_SEND);
    emailIntent.setType("text/rtf");
    emailIntent.putExtra(android.content.Intent.EXTRA_SUBJECT, "OwnCloud Logfile");
    emailIntent.putExtra(android.content.Intent.EXTRA_TEXT, "This is a automatic E-mail send by owncloud/android");
    emailIntent.putExtra(android.content.Intent.EXTRA_STREAM, fileUri);
    emailIntent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
    context.startActivity(Intent.createChooser(emailIntent, "Send mail..."));
    }
    });
    return rowView;
    }
    }

  • These two classes seem to not be used anymore, so both, and the package thire in can be removed:

    public MenuParcelable() {
    mMenuItems = new ArrayList<MenuItemParcelable>();
    }
    public MenuParcelable(Parcel in) {
    in.readTypedList(mMenuItems, MenuItemParcelable.CREATOR);
    }
    public static final Parcelable.Creator<MenuParcelable> CREATOR = new Parcelable.Creator<MenuParcelable>() {
    @Override
    public MenuParcelable createFromParcel(Parcel in) {
    return new MenuParcelable(in);
    }
    @Override
    public MenuParcelable[] newArray(int size) {
    return new MenuParcelable[size];
    }
    };
    @Override
    public int describeContents() {
    return 0;
    }
    @Override
    public void writeToParcel(Parcel outParcel, int flags) {
    outParcel.writeTypedList(mMenuItems);
    }
    }

    public void setMenuItemId(int id) {
    mMenuItemId = id;
    }
    public int getMenuItemId() {
    return mMenuItemId;
    }
    public String getMenuText() {
    return mMenuText;
    }
    public void setMenuText(String mMenuText) {
    this.mMenuText = mMenuText;
    }
    public static final Parcelable.Creator<MenuItemParcelable> CREATOR =
    new Parcelable.Creator<MenuItemParcelable>() {
    @Override
    public MenuItemParcelable createFromParcel(Parcel source) {
    return new MenuItemParcelable(source);
    }
    @Override
    public MenuItemParcelable[] newArray(int size) {
    return new MenuItemParcelable[size];
    }
    };
    @Override
    public int describeContents() {
    return 0;
    }
    @Override
    public void writeToParcel(Parcel dest, int flags) {
    dest.writeInt(mMenuItemId);
    }
    }

  • Lol

    package com.owncloud.android.ui.fragment;
    import android.support.v4.app.Fragment;
    public class AuthenticatorAccountDetailsFragment extends Fragment {
    }

  • /**
    * Constructor
    *
    * @param anchor {@link View} on where the popup window should be displayed
    */
    public QuickAction(View anchor) {
    super(anchor);
    actionList = new ArrayList<ActionItem>();
    context = anchor.getContext();
    inflater = (LayoutInflater) context
    .getSystemService(Context.LAYOUT_INFLATER_SERVICE);
    root = (ViewGroup) inflater.inflate(R.layout.popup, null);
    mArrowDown = (ImageView) root.findViewById(R.id.arrow_down);
    mArrowUp = (ImageView) root.findViewById(R.id.arrow_up);
    setContentView(root);
    mTrack = (ViewGroup) root.findViewById(R.id.tracks);
    scroller = (ScrollView) root.findViewById(R.id.scroller);
    animStyle = ANIM_AUTO;
    }

  • lol nr. 2

    package com.owncloud.android.ui.fragment;
    import android.support.v4.app.Fragment;
    public class AuthenticatorGetStartedFragment extends Fragment {
    }

  • package com.owncloud.android.utils;
    import android.content.Intent;
    import android.graphics.drawable.Drawable;
    import android.view.ActionProvider;
    import android.view.ContextMenu;
    import android.view.MenuItem;
    import android.view.SubMenu;
    import android.view.View;
    /**
    * Created by scherzia on 17.08.2015.
    */
    public class DialogMenuItem implements MenuItem {
    int mItemId;
    CharSequence mTitle;
    public DialogMenuItem(int itemId) {
    this.mItemId = itemId;
    }
    @Override
    public int getItemId() {
    return mItemId;
    }
    @Override
    public int getGroupId() {
    return 0;
    }
    @Override
    public int getOrder() {
    return 0;
    }
    @Override
    public MenuItem setTitle(CharSequence title) {
    this.mTitle = title;
    return this;
    }
    @Override
    public MenuItem setTitle(int title) {
    return this;
    }
    @Override
    public CharSequence getTitle() {
    return this.mTitle;
    }
    @Override
    public MenuItem setTitleCondensed(CharSequence title) {
    return null;
    }
    @Override
    public CharSequence getTitleCondensed() {
    return null;
    }
    @Override
    public MenuItem setIcon(Drawable icon) {
    return null;
    }
    @Override
    public MenuItem setIcon(int iconRes) {
    return null;
    }
    @Override
    public Drawable getIcon() {
    return null;
    }
    @Override
    public MenuItem setIntent(Intent intent) {
    return null;
    }
    @Override
    public Intent getIntent() {
    return null;
    }
    @Override
    public MenuItem setShortcut(char numericChar, char alphaChar) {
    return null;
    }
    @Override
    public MenuItem setNumericShortcut(char numericChar) {
    return null;
    }
    @Override
    public char getNumericShortcut() {
    return 0;
    }
    @Override
    public MenuItem setAlphabeticShortcut(char alphaChar) {
    return null;
    }
    @Override
    public char getAlphabeticShortcut() {
    return 0;
    }
    @Override
    public MenuItem setCheckable(boolean checkable) {
    return null;
    }
    @Override
    public boolean isCheckable() {
    return false;
    }
    @Override
    public MenuItem setChecked(boolean checked) {
    return null;
    }
    @Override
    public boolean isChecked() {
    return false;
    }
    @Override
    public MenuItem setVisible(boolean visible) {
    return null;
    }
    @Override
    public boolean isVisible() {
    return false;
    }
    @Override
    public MenuItem setEnabled(boolean enabled) {
    return null;
    }
    @Override
    public boolean isEnabled() {
    return false;
    }
    @Override
    public boolean hasSubMenu() {
    return false;
    }
    @Override
    public SubMenu getSubMenu() {
    return null;
    }
    @Override
    public MenuItem setOnMenuItemClickListener(OnMenuItemClickListener menuItemClickListener) {
    return null;
    }
    @Override
    public ContextMenu.ContextMenuInfo getMenuInfo() {
    return null;
    }
    @Override
    public void setShowAsAction(int actionEnum) {
    }
    @Override
    public MenuItem setShowAsActionFlags(int actionEnum) {
    return null;
    }
    @Override
    public MenuItem setActionView(View view) {
    return null;
    }
    @Override
    public MenuItem setActionView(int resId) {
    return null;
    }
    @Override
    public View getActionView() {
    return null;
    }
    @Override
    public MenuItem setActionProvider(ActionProvider actionProvider) {
    return null;
    }
    @Override
    public ActionProvider getActionProvider() {
    return null;
    }
    @Override
    public boolean expandActionView() {
    return false;
    }
    @Override
    public boolean collapseActionView() {
    return false;
    }
    @Override
    public boolean isActionViewExpanded() {
    return false;
    }
    @Override
    public MenuItem setOnActionExpandListener(OnActionExpandListener listener) {
    return null;
    }
    }

  • package com.owncloud.android.utils;
    import android.content.Context;
    import android.content.Intent;
    import android.content.IntentFilter;
    import android.net.ConnectivityManager;
    import android.net.NetworkInfo.State;
    import android.os.BatteryManager;
    public class UploadUtils {
    public static boolean isCharging(Context context) {
    IntentFilter ifilter = new IntentFilter(Intent.ACTION_BATTERY_CHANGED);
    Intent batteryStatus = context.registerReceiver(null, ifilter);
    int status = batteryStatus.getIntExtra(BatteryManager.EXTRA_STATUS, -1);
    return status == BatteryManager.BATTERY_STATUS_CHARGING || status == BatteryManager.BATTERY_STATUS_FULL;
    }
    public static boolean isOnline(Context context) {
    ConnectivityManager cm = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
    return cm.getActiveNetworkInfo() != null && cm.getActiveNetworkInfo().isConnected();
    }
    public static boolean isConnectedViaWiFi(Context context) {
    ConnectivityManager cm = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
    return cm != null && cm.getActiveNetworkInfo() != null
    && cm.getActiveNetworkInfo().getType() == ConnectivityManager.TYPE_WIFI
    && cm.getActiveNetworkInfo().getState() == State.CONNECTED;
    }
    }

  • import android.content.Context;
    import android.preference.CheckBoxPreference;
    import android.view.View;
    import com.owncloud.android.R;
    public class RadioButtonPreference extends CheckBoxPreference implements View.OnLongClickListener {
    public RadioButtonPreference(Context context) {
    super(context, null, android.R.attr.checkBoxPreferenceStyle);
    setWidgetLayoutResource(R.layout.preference_widget_radiobutton);
    }
    @Override
    public boolean onLongClick(View v) {
    return true;
    }
    }

Methods not in use anymore

  • public static class UnsupportedFeaturesException extends
    AuthenticatorException {
    public static final long serialVersionUID = 1L;
    public UnsupportedFeaturesException() {
    super(AccountManager.ERROR_CODE_UNSUPPORTED_OPERATION,
    "Unsupported features");
    }
    }
    public static class AccessDeniedException extends AuthenticatorException {
    public AccessDeniedException(int code, String errorMsg) {
    super(AccountManager.ERROR_CODE_INVALID_RESPONSE, "Access Denied");
    }
    private static final long serialVersionUID = 1L;
    }
  • [WON'T BE DELETED, IT MAKES SENSE TO KEEP THEM]
    public boolean containsKey( String key ) {
    boolean contained = false;
    DiskLruCache.Snapshot snapshot = null;
    String validKey = convertToValidKey(key);
    try {
    snapshot = mDiskCache.get( validKey );
    contained = snapshot != null;
    } catch (IOException e) {
    e.printStackTrace();
    } finally {
    if ( snapshot != null ) {
    snapshot.close();
    }
    }
    return contained;
    }
    public void clearCache() {
    if ( BuildConfig.DEBUG ) {
    Log_OC.d( "cache_test_DISK_", "disk cache CLEARED");
    }
    try {
    mDiskCache.delete();
    } catch ( IOException e ) {
    e.printStackTrace();
    }
    }
  • private void wifiDisconnected(Context context) {
    // TODO something smart
    // NOTE: explicit cancellation of only-wifi camera uploads is not needed anymore, since currently:
    // - any upload in progress will be interrupted due to the lack of connectivity while the device
    // reconnects through other network interface;
    // - FileUploader checks camera upload settings and connection state before executing each
    // upload operation, so other pending camera uploads after the current one will not be run
    // (currently are silently moved to FAILED state)
    }
    static public void enableActionReceiver(Context context) {
    PackageManager pm = context.getPackageManager();
    ComponentName compName = new ComponentName(context.getApplicationContext(), ConnectivityActionReceiver.class);
    pm.setComponentEnabledSetting(compName, PackageManager.COMPONENT_ENABLED_STATE_ENABLED,
    PackageManager.DONT_KILL_APP);
    }
    static public void disableActionReceiver(Context context) {
    PackageManager pm = context.getPackageManager();
    ComponentName compName = new ComponentName(context.getApplicationContext(), ConnectivityActionReceiver.class);
    pm.setComponentEnabledSetting(compName, PackageManager.COMPONENT_ENABLED_STATE_DISABLED,
    PackageManager.DONT_KILL_APP);
    }
  • public FeatureItem(int image, int titleText, int contentText, String version, String betaVersion) {
    this(image, titleText, contentText, version, betaVersion, false);
    }
  • [wont fix because it may make sense to keep it here]
    private void sendBroadcastOperationFinished(Target target, RemoteOperation operation,
    RemoteOperationResult result) {
    Intent intent = new Intent(ACTION_OPERATION_FINISHED);
    intent.putExtra(EXTRA_RESULT, result);
    if (target.mAccount != null) {
    intent.putExtra(EXTRA_ACCOUNT, target.mAccount);
    } else {
    intent.putExtra(EXTRA_SERVER_URL, target.mServerUrl);
    }
    mLocalBroadcastManager.sendBroadcast(intent);
    }
  • public void setSupportActionBar(@Nullable Toolbar toolbar) {
    getDelegate().setSupportActionBar(toolbar);
    }
  • https://github.com/owncloud/android-library/blob/11363adedb00211c321ba7ed2db8dd1fcc2421c9/src/com/owncloud/android/lib/common/network/AdvancedSslSocketFactory.java#L104-L142
  • https://github.com/owncloud/android-library/blob/e466bac6b15336983340e8b2c552a88423a06264/src/com/owncloud/android/lib/common/SimpleFactoryManager.java#L71-L80

Things to REFACTOR

  • Public declaration in several interfaces can be removed, since interface methods are always public.
  • Actively converting UI elements is not necessary anymore. This these can be Removed. Like here
  • These lines can be simplified
    file.setSharedViaLink(c.getInt(
    c.getColumnIndex(ProviderTableMeta.FILE_SHARED_VIA_LINK)) == 1 ? true : false);
    file.setSharedWithSharee(c.getInt(
    c.getColumnIndex(ProviderTableMeta.FILE_SHARED_WITH_SHAREE)) == 1 ? true : false);
    file.setPermissions(c.getString(c.getColumnIndex(ProviderTableMeta.FILE_PERMISSIONS)));
    file.setRemoteId(c.getString(c.getColumnIndex(ProviderTableMeta.FILE_REMOTE_ID)));
    file.setNeedsUpdateThumbnail(c.getInt(
    c.getColumnIndex(ProviderTableMeta.FILE_UPDATE_THUMBNAIL)) == 1 ? true : false);
    file.setDownloading(c.getInt(
    c.getColumnIndex(ProviderTableMeta.FILE_IS_DOWNLOADING)) == 1 ? true : false);
  • DELETE unused imports like here
  • these two functions can be reduced to one big, and one small. Also we should consider to remove them since they are not in use:
    public static boolean cancelPotentialAvatarWork(Object file, ImageView imageView) {
    final GetAvatarTask avatarWorkerTask = getAvatarWorkerTask(imageView);
    if (avatarWorkerTask != null) {
    final Object usernameData = avatarWorkerTask.mUsername;
    // If usernameData is not yet set or it differs from the new data
    if (usernameData == null || usernameData != file) {
    // Cancel previous task
    avatarWorkerTask.cancel(true);
    Log_OC.v(TAG, "Cancelled generation of avatar for a reused imageView");
    } else {
    // The same work is already in progress
    return false;
    }
    }
    // No task associated with the ImageView, or an existing task was cancelled
    return true;
    }
    public static boolean cancelPotentialAvatarWork(Object file, MenuItem menuItem) {
    final GetAvatarTask avatarWorkerTask = getAvatarWorkerTask(menuItem);
    if (avatarWorkerTask != null) {
    final Object usernameData = avatarWorkerTask.mUsername;
    // If usernameData is not yet set or it differs from the new data
    if (usernameData == null || usernameData != file) {
    // Cancel previous task
    avatarWorkerTask.cancel(true);
    Log_OC.v(TAG, "Cancelled generation of avatar for a reused imageView");
    } else {
    // The same work is already in progress
    return false;
    }
    }
    // No task associated with the ImageView, or an existing task was cancelled
    return true;
    }
  • FutureList has a package for it's own. Maybe it can be moved somewhere else.
  • Lets do something or remove this:
    @Override
    public void onFinishInflate() {
    /*
    if (mRoot != null)
    initControllerView(mRoot);
    */
    }
    /* TODO REMOVE
    public MediaControlView(Context context, boolean useFastForward) {
    super(context);
    mContext = context;
    mUseFastForward = useFastForward;
    initFloatingWindowLayout();
    //initFloatingWindow();
    }
    */
    /* TODO REMOVE
    public MediaControlView(Context context) {
    this(context, true);
    }
    */
    /* T
    private void initFloatingWindow() {
    mWindowManager = (WindowManager)mContext.getSystemService(Context.WINDOW_SERVICE);
    mWindow = PolicyManager.makeNewWindow(mContext);
    mWindow.setWindowManager(mWindowManager, null, null);
    mWindow.requestFeature(Window.FEATURE_NO_TITLE);
    mDecor = mWindow.getDecorView();
    mDecor.setOnTouchListener(mTouchListener);
    mWindow.setContentView(this);
    mWindow.setBackgroundDrawableResource(android.R.color.transparent);
    // While the media controller is up, the volume control keys should
    // affect the media stream type
    mWindow.setVolumeControlStream(AudioManager.STREAM_MUSIC);
    setFocusable(true);
    setFocusableInTouchMode(true);
    setDescendantFocusability(ViewGroup.FOCUS_AFTER_DESCENDANTS);
    requestFocus();
    }
    */
    /*
    // Allocate and initialize the static parts of mDecorLayoutParams. Must
    // also call updateFloatingWindowLayout() to fill in the dynamic parts
    // (y and width) before mDecorLayoutParams can be used.
    private void initFloatingWindowLayout() {
    mDecorLayoutParams = new WindowManager.LayoutParams();
    WindowManager.LayoutParams p = mDecorLayoutParams;
    p.gravity = Gravity.TOP;
    p.height = LayoutParams.WRAP_CONTENT;
    p.x = 0;
    p.format = PixelFormat.TRANSLUCENT;
    p.type = WindowManager.LayoutParams.TYPE_APPLICATION_PANEL;
    p.flags |= WindowManager.LayoutParams.FLAG_ALT_FOCUSABLE_IM
    | WindowManager.LayoutParams.FLAG_NOT_TOUCH_MODAL
    | WindowManager.LayoutParams.FLAG_SPLIT_TOUCH;
    p.token = null;
    p.windowAnimations = 0; // android.R.style.DropDownAnimationDown;
    }
    */
    // Update the dynamic parts of mDecorLayoutParams
    // Must be called with mAnchor != NULL.
    /*
    private void updateFloatingWindowLayout() {
    int [] anchorPos = new int[2];
    mAnchor.getLocationOnScreen(anchorPos);
    WindowManager.LayoutParams p = mDecorLayoutParams;
    p.width = mAnchor.getWidth();
    p.y = anchorPos[1] + mAnchor.getHeight();
    }
    */
    /*
    // This is called whenever mAnchor's layout bound changes
    public void onLayoutChange(View v, int left, int top, int right,
    int bottom, int oldLeft, int oldTop, int oldRight,
    int oldBottom) {
    //updateFloatingWindowLayout();
    if (mShowing) {
    mWindowManager.updateViewLayout(mDecor, mDecorLayoutParams);
    }
    }
    */
    /*
    public boolean onTouch(View v, MotionEvent event) {
    if (event.getAction() == MotionEvent.ACTION_DOWN) {
    if (mShowing) {
    hide();
    }
    }
    return false;
    }
    */
  • Remove commends:
    /*
    Uri folderUri = ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, Long.parseLong(uri.getPathSegments().get(1)));
    Cursor folder = query(db, folderUri, null, null, null, null);
    String folderName = "(unknown)";
    if (folder != null && folder.moveToFirst()) {
    folderName = folder.getString(folder.getColumnIndex(ProviderTableMeta.FILE_PATH));
    }
    */
    Cursor children = query(uri, null, null, null, null);
    if (children != null && children.moveToFirst()) {
    long childId;
    boolean isDir;
    while (!children.isAfterLast()) {
    childId = children.getLong(children.getColumnIndex(ProviderTableMeta._ID));
    isDir = "DIR".equals(children.getString(
    children.getColumnIndex(ProviderTableMeta.FILE_CONTENT_TYPE)
    ));
    //remotePath = children.getString(children.getColumnIndex(ProviderTableMeta.FILE_PATH));
    if (isDir) {
    count += delete(
    db,
    ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_DIR, childId),
    null,
    null
    );
    } else {
    count += delete(
    db,
    ContentUris.withAppendedId(ProviderTableMeta.CONTENT_URI_FILE, childId),
    null,
    null
    );
    }
    children.moveToNext();
    }
    children.close();
    } /*else {
    Log_OC.d(TAG, "No child to remove in DIRECTORY " + folderName);
    }
    Log_OC.d(TAG, "Removing DIRECTORY " + folderName + " (or maybe not) ");
    */
    count += db.delete(ProviderTableMeta.FILE_TABLE_NAME,
    ProviderTableMeta._ID
    + "="
    + uri.getPathSegments().get(1)
    + (!TextUtils.isEmpty(where) ? " AND (" + where
    + ")" : ""), whereArgs);
    /* Just for log
    if (folder != null) {
    folder.close();
    }*/
  • OnEnorceableRefreshListener can be embeded into ExtendedListFragment
  • Enum does not need to be static
    public static enum Decision {
    CANCEL,
    KEEP_BOTH,
    OVERWRITE,
    SERVER
    }
  • ifles can be replaces with switch statements
    https://github.com/owncloud/android/blob/426bfefe9db7f1ad4aea47a03cc1009bf1e2323e/src/com/owncloud/android/ui/errorhandling/ErrorMessageAdapter.java#L21-L481
  • Replace all this getter functions with simple findActionById() calls or final object declarations
    /**
    * Show error when creating or updating the public share, if any
    * @param errorMessage
    */
    public void showError(String errorMessage) {
    getErrorMessage().setVisibility(View.VISIBLE);
    getErrorMessage().setText(errorMessage);
    }
    private TextView getDialogTitle(View view) {
    return (TextView) view.findViewById(R.id.publicShareDialogTitle);
    }
    private LinearLayout getNameSection(View view) {
    return (LinearLayout) view.findViewById(R.id.shareViaLinkNameSection);
    }
    private EditText getNameValue(View view) {
    return (EditText) view.findViewById(R.id.shareViaLinkNameValue);
    }
    private View getEditPermissionSection(View view) {
    return view.findViewById(R.id.shareViaLinkEditPermissionSection);
    }
    private View getShowFileListingSection(View view) {
    return view.findViewById(R.id.shareViaShowFileListingSection);
    }
    private SwitchCompat getEditPermissionSwitch(View view) {
    return (SwitchCompat) view.findViewById(R.id.shareViaLinkEditPermissionSwitch);
    }
    private SwitchCompat getShowFileListingSwitch(View view) {
    return (SwitchCompat) view.findViewById(R.id.shareViaShowFileListingSwitch);
    }
    private TextView getPasswordLabel(View view) {
    return (TextView) view.findViewById(R.id.shareViaLinkPasswordLabel);
    }
    private SwitchCompat getPasswordSwitch(View view) {
    return (SwitchCompat) view.findViewById(R.id.shareViaLinkPasswordSwitch);
    }
    private void setPasswordSwitchChecked(boolean checked, View view) {
    SwitchCompat theSwitch = getPasswordSwitch(view);
    theSwitch.setOnCheckedChangeListener(null);
    theSwitch.setChecked(checked);
    theSwitch.setOnCheckedChangeListener(mOnPasswordInteractionListener);
    }
    private TextView getPasswordValue(View view) {
    return (TextView) view.findViewById(R.id.shareViaLinkPasswordValue);
    }
    private TextView getExpirationDateLabel(View view) {
    return (TextView) view.findViewById(R.id.shareViaLinkExpirationLabel);
    }
    private SwitchCompat getExpirationDateSwitch(View view) {
    return (SwitchCompat) view.findViewById(R.id.shareViaLinkExpirationSwitch);
    }
    private void setExpirationDateSwitchChecked(boolean checked, View view) {
    SwitchCompat theSwitch = getExpirationDateSwitch(view);
    theSwitch.setOnCheckedChangeListener(null);
    theSwitch.setChecked(checked);
    theSwitch.setOnCheckedChangeListener(mOnExpirationDateInteractionListener);
    }
    private TextView getExpirationDateValue(View view) {
    return (TextView) view.findViewById(R.id.shareViaLinkExpirationValue);
    }
    private TextView getExpirationDateExplanation(View view) {
    return (TextView) view.findViewById(R.id.shareViaLinkExpirationExplanationLabel);
    }
    private TextView getErrorMessage() {
    return (TextView) getView().findViewById(R.id.public_link_error_message);
    }
  • Should be moved into the utils package
    import android.app.NotificationManager;
    import android.content.Context;
    import android.os.Handler;
    import android.os.HandlerThread;
    import android.os.Process;
    import android.support.v4.app.NotificationCompat;
    import com.owncloud.android.R;
    import java.util.Random;
    public class NotificationUtils {
    /**
    * Factory method for {@link android.support.v4.app.NotificationCompat.Builder} instances.
    *
    * Not strictly needed from the moment when the minimum API level supported by the app
    * was raised to 14 (Android 4.0).
    *
    * Formerly, returned a customized implementation of {@link android.support.v4.app.NotificationCompat.Builder}
    * for Android API levels >= 8 and < 14.
    *
    * Kept in place for the extra abstraction level; notifications in the app need a review, and they
    * change a lot in different Android versions.
    *
    * @param context Context that will use the builder to create notifications
    * @return An instance of the regular {@link NotificationCompat.Builder}.
    */
    public static NotificationCompat.Builder newNotificationBuilder(Context context) {
    return new NotificationCompat.Builder(context).
    setColor(context.getResources().getColor(R.color.primary));
    }
    public static void cancelWithDelay(
    final NotificationManager notificationManager,
    final int notificationId,
    long delayInMillis) {
    HandlerThread thread = new HandlerThread(
    "NotificationDelayerThread_" + (new Random(System.currentTimeMillis())).nextInt(),
    Process.THREAD_PRIORITY_BACKGROUND);
    thread.start();
    Handler handler = new Handler(thread.getLooper());
    handler.postDelayed(new Runnable() {
    public void run() {
    notificationManager.cancel(notificationId);
    ((HandlerThread)Thread.currentThread()).getLooper().quit();
    }
    }, delayInMillis);
    }
    }
  • Do we still need this? If it was important for us later we could still checkout a n older version of the app, so I would vote for no.
    /* -*
    * Called when a change in the shown pages is going to start being made.
    *
    * @param container The containing View which is displaying this adapter's page views.
    *- /
    @Override
    public void startUpdate(ViewGroup container) {
    Log_OC.e(TAG, "** startUpdate");
    }
    @Override
    public Object instantiateItem(ViewGroup container, int position) {
    Log_OC.e(TAG, "** instantiateItem " + position);
    if (mFragments.size() > position) {
    Fragment fragment = mFragments.get(position);
    if (fragment != null) {
    Log_OC.e(TAG, "** \t returning cached item");
    return fragment;
    }
    }
    if (mCurTransaction == null) {
    mCurTransaction = mFragmentManager.beginTransaction();
    }
    Fragment fragment = getItem(position);
    if (mSavedState.size() > position) {
    Fragment.SavedState savedState = mSavedState.get(position);
    if (savedState != null) {
    // TODO WATCH OUT:
    // * The Fragment must currently be attached to the FragmentManager.
    // * A new Fragment created using this saved state must be the same class type as the Fragment it was created from.
    // * The saved state can not contain dependencies on other fragments -- that is it can't use putFragment(Bundle, String, Fragment)
    // to store a fragment reference
    fragment.setInitialSavedState(savedState);
    }
    }
    while (mFragments.size() <= position) {
    mFragments.add(null);
    }
    fragment.setMenuVisibility(false);
    mFragments.set(position, fragment);
    //Log_OC.e(TAG, "** \t adding fragment at position " + position + ", containerId " + container.getId());
    mCurTransaction.add(container.getId(), fragment);
    return fragment;
    }
    @Override
    public void destroyItem(ViewGroup container, int position, Object object) {
    Log_OC.e(TAG, "** destroyItem " + position);
    Fragment fragment = (Fragment)object;
    if (mCurTransaction == null) {
    mCurTransaction = mFragmentManager.beginTransaction();
    }
    Log_OC.e(TAG, "** \t removing fragment at position " + position);
    while (mSavedState.size() <= position) {
    mSavedState.add(null);
    }
    mSavedState.set(position, mFragmentManager.saveFragmentInstanceState(fragment));
    mFragments.set(position, null);
    mCurTransaction.remove(fragment);
    }
    @Override
    public void setPrimaryItem(ViewGroup container, int position, Object object) {
    Fragment fragment = (Fragment)object;
    if (fragment != mCurrentPrimaryItem) {
    if (mCurrentPrimaryItem != null) {
    mCurrentPrimaryItem.setMenuVisibility(false);
    }
    if (fragment != null) {
    fragment.setMenuVisibility(true);
    }
    mCurrentPrimaryItem = fragment;
    }
    }
    @Override
    public void finishUpdate(ViewGroup container) {
    Log_OC.e(TAG, "** finishUpdate (start)");
    if (mCurTransaction != null) {
    mCurTransaction.commitAllowingStateLoss();
    mCurTransaction = null;
    mFragmentManager.executePendingTransactions();
    }
    Log_OC.e(TAG, "** finishUpdate (end)");
    }
    @Override
    public boolean isViewFromObject(View view, Object object) {
    return ((Fragment)object).getView() == view;
    }
    @Override
    public Parcelable saveState() {
    Bundle state = null;
    if (mSavedState.size() > 0) {
    state = new Bundle();
    Fragment.SavedState[] savedStates = new Fragment.SavedState[mSavedState.size()];
    mSavedState.toArray(savedStates);
    state.putParcelableArray("states", savedStates);
    }
    for (int i=0; i<mFragments.size(); i++) {
    Fragment fragment = mFragments.get(i);
    if (fragment != null) {
    if (state == null) {
    state = new Bundle();
    }
    String key = "f" + i;
    mFragmentManager.putFragment(state, key, fragment);
    }
    }
    return state;
    }
    @Override
    public void restoreState(Parcelable state, ClassLoader loader) {
    if (state != null) {
    Bundle bundle = (Bundle)state;
    bundle.setClassLoader(loader);
    Parcelable[] states = bundle.getParcelableArray("states");
    mSavedState.clear();
    mFragments.clear();
    if (states != null) {
    for (int i=0; i<states.length; i++) {
    mSavedState.add((Fragment.SavedState)states[i]);
    }
    }
    Iterable<String> keys = bundle.keySet();
    for (String key: keys) {
    if (key.startsWith("f")) {
    int index = Integer.parseInt(key.substring(1));
    Fragment f = mFragmentManager.getFragment(bundle, key);
    if (f != null) {
    while (mFragments.size() <= index) {
    mFragments.add(null);
    }
    f.setMenuVisibility(false);
    mFragments.set(index, f);
    } else {
    Log_OC.w(TAG, "Bad fragment at key " + key);
    }
    }
    }
    }
    }
    */
  • Remove all the new lines
    public static PreviewVideoError handlePreviewVideoError(ExoPlaybackException error,
    Context context) {
    PreviewVideoError previewVideoError = null;
    switch (error.type) {
    case ExoPlaybackException.TYPE_SOURCE:
    previewVideoError = handlePlayerSourceError(error, context);
    break;
    case ExoPlaybackException.TYPE_UNEXPECTED:
    previewVideoError = handlePlayerUnexpectedError(error, context);
    break;
    case ExoPlaybackException.TYPE_RENDERER:
    previewVideoError = handlePlayerRendererError(error, context);
    break;
    }
    return previewVideoError;
    }
  • Method should call super
    @Override
    public void onConfigurationChanged(Configuration newConfig) {
    Log_OC.v(TAG, "onConfigurationChanged " + this);
    }
  • Class should not have its own package
    public class ProgressIndicator extends FrameLayout {
  • Should use AppCombatImageView
    public class SquareImageView extends ImageView {
  • If/else can be replaced by switch
    if (orientation == ExifInterface.ORIENTATION_FLIP_HORIZONTAL)
    {
    matrix.postScale(-1.0f, 1.0f);
    }
    // 3
    else if (orientation == ExifInterface.ORIENTATION_ROTATE_180)
    {
    matrix.postRotate(180);
    }
    // 4
    else if (orientation == ExifInterface.ORIENTATION_FLIP_VERTICAL)
    {
    matrix.postScale(1.0f, -1.0f);
    }
    // 5
    else if (orientation == ExifInterface.ORIENTATION_TRANSPOSE)
    {
    matrix.postRotate(-90);
    matrix.postScale(1.0f, -1.0f);
    }
    // 6
    else if (orientation == ExifInterface.ORIENTATION_ROTATE_90)
    {
    matrix.postRotate(90);
    }
    // 7
    else if (orientation == ExifInterface.ORIENTATION_TRANSVERSE)
    {
    matrix.postRotate(90);
    matrix.postScale(1.0f, -1.0f);
    }
    // 8
    else if (orientation == ExifInterface.ORIENTATION_ROTATE_270)
    {
    matrix.postRotate(270);
    }
  • [nofix] Class can be replaced by the two lines it contains ... actually thinking about this twice it makes sense to have this class
    package com.owncloud.android.utils;
    import android.content.Context;
    import android.os.Build;
    import android.os.PowerManager;
    public class PowerUtils {
    public static boolean isDeviceIdle(Context context) {
    return (
    Build.VERSION.SDK_INT >= Build.VERSION_CODES.M &&
    ((PowerManager)context.getSystemService(Context.POWER_SERVICE)).isDeviceIdleMode()
    );
    }
    }
  • Should be replaced by AppCombatEditText
    public class ActionEditText extends EditText {
  • Exceptions can be removed
    https://github.com/owncloud/android-library/blob/11363adedb00211c321ba7ed2db8dd1fcc2421c9/src/com/owncloud/android/lib/common/network/AdvancedSslSocketFactory.java#L158-L162
  • Can be replaces by switch case
    https://github.com/owncloud/android-library/blob/ee726119cb21ac07c17857cb1a81baa262bf42da/src/com/owncloud/android/lib/resources/shares/ShareXMLParser.java#L296-L369
  • @davivel according to git anode you added it, but what is the purpose of this class?
    public abstract class HookActivity extends FileActivity {
    private static final String TAG = HookActivity.class.getName();
    }
  • remove unnecessary commends https://github.com/owncloud/android/blob/master/src/com/owncloud/android/ui/fragment/PublicShareDialogFragment.java#L193-L361

Need despaget

@theScrabi
Copy link
Contributor Author

Also we might want to consider to upgrade to Java 8 since that comes very in handy with lambda expressions, which is good because less ugly Java code -> better readable.

@dutta14
Copy link

dutta14 commented Mar 18, 2018

I can work on this if it has not been taken up yet.

@theScrabi
Copy link
Contributor Author

@dutta14 if you like go ahead :)

This was referenced Apr 10, 2018
@davigonz davigonz added this to the 2.8.0 milestone Apr 10, 2018
@jesmrec jesmrec added the Epic label Jun 20, 2018
@theScrabi theScrabi modified the milestones: 2.8.0, 2.9.0 Jun 25, 2018
@jesmrec jesmrec removed this from the 2.9.0 milestone Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants