Refactoring Overall files #2786

rubinstory · 2022-05-22T14:46:35Z

Refactored Overall project files from Pusan National University.

test: git test with change readme

MethodInvokeResult Since FieldInvokeResult and MethodInvokeResult class has a lot in common, extracting them as superclass seems to be more useful for adding other "InvokeResult" classes in the future.

Added|Fixed: Extract Superclass from FieldInvokeResult and

Considering a boolean return type and CamelCase style, changed the function name to isInTable.

According to the code point, the cases where the width is 0 are defined. For clarity, Renamed function variable `ucs` to `codePoint`.

Extract getSoftwareInfoMarkDownString(), getHardwareInfoMarkdownString() from getDeviceInfoMarkdownString() due to the complex structure of the method. getSoft...String() returns information of device software in markdown language. getHard...String() returns information of device hardware in markdown language.

refactor: Extract methods from getDeviceInfoMarkdownString()

Refactor/wc width

Add isString, isEmpty, isREL boolean variables to the method. Now we can check the if statement's condition more easily.

refactor: Add Explaining Variables at appendPropertyToMarkdownIfSet()

Changed: Refator `WCWidth.java`

Considering a boolean return type and CamelCase style, changed the function name to isInTable.

According to the code point, the cases where the width is 0 are defined. For clarity, Renamed function variable `ucs` to `codePoint`.

result != PERMISSION_GRANTED -> boolean permissionGranted context instanceof App..Activity -> boolean isApp..Activity context instanceof Activity -> booelean isActivity

Replaced Error Codes 0 with IllegalArgumentException by each case.

Extract hasBootstrapError() from setupBootstrapIfNeeded() due to the duplicate code of the method

Refactor/terminal view

…`parse()` Include only the parts that make sense for the try-catch.

The code has two methods and variable name isn't good. so it needs to refactor codes. `getAction`, `chooseAction` are extracted. The code is figured out easily to rename variables

…dable

…) for more readable

In `tryParseColor()` from `TerminalColors.java` and `updateWith()` from `TerminalColorScheme.java`, setting parsed color is wrapped by try-catch. In `TerminalTest.java`, wrong input testing code is wrapped by try-catch.

Refactor/terminal colors

…ent.java` `configureTermuxAPIPreference()`, `configureTermuxFloatPreference()`, etc are very similar method. so Using Template method reduces overlapped codes. At first, make abstract class

Changed: Apply State pattern in `TerminalEmulator`

Apply Template method for `configureTermuxPreference` and delete orginal code about `configureTermuxAPIPreference()`

…ent.java Added: Add `configureTermuxPreference.java` for `RootPreferencesFragment.java`

Changed: Apply State pattern in `TerminalEmulator`

…eference.java Added: extract class `configureTermuxAPIPreference()`

agnostic-apollo · 2022-05-29T15:22:45Z

app/src/main/java/com/termux/app/TermuxService.java

    @Override
    public void onTermuxSessionExited(final TermuxSession termuxSession) {
-        if (termuxSession != null) {
+        boolean isTermuxSession = termuxSession != null;


These and all the other if conditions extracted to boolean are useless and server no purpose, using an if condition already implies that one or more conditions will be supplied in it. Do not do this. If there are multiple conditions that are too complex, they can be split on multiple lines, in some case extraction to boolean is indeed fine.

agnostic-apollo · 2022-05-29T15:33:52Z

Do not modify the SettingsActivity, RootPreferencesFragment and the other PreferenceDataStore, or move them to individual class file. I kept the fragments and datastores in same classes since they served similar functionalities and splitting would just create too many files without any need.

agnostic-apollo · 2022-05-29T16:24:25Z

termux-shared/src/main/java/com/termux/shared/logger/Logger.java

-            Log.d(getFullTag(tag), message);
-        else if (logPriority == Log.VERBOSE && CURRENT_LOG_LEVEL >= LOG_LEVEL_VERBOSE)
-            Log.v(getFullTag(tag), message);
+        boolean higherThanNormal = (CURRENT_LOG_LEVEL >= LOG_LEVEL_NORMAL);


Don't do this. Need to minimize if conditions calculated for log statements.

agnostic-apollo · 2022-05-29T16:26:01Z

termux-shared/src/main/java/com/termux/shared/settings/preferences/SharedPreferenceUtils.java

     */
    public static boolean getBoolean(SharedPreferences sharedPreferences, String key, boolean def) {
-        if (sharedPreferences == null) {
+        if (isSharedPreferenceNull(sharedPreferences)) {


Don't do this.

agnostic-apollo · 2022-05-29T16:27:17Z

termux-shared/src/main/java/com/termux/shared/termux/TermuxConstants.java

    /** Termux and plugin apps directory */
    public static final File TERMUX_APPS_DIR = new File(TERMUX_APPS_DIR_PATH);

+    public static StringBuilder buildScript(String filesDir) {


Don't do this. Not the place for scripts.

agnostic-apollo · 2022-05-29T16:27:31Z

terminal-view/src/main/java/com/termux/view/TerminalView.java

            public boolean onUp(MotionEvent event) {
                mScrollRemainder = 0.0f;
-                if (mEmulator != null && mEmulator.isMouseTrackingActive() && !event.isFromSource(InputDevice.SOURCE_MOUSE) && !isSelectingText() && !scrolledWithFinger) {
+                if (!isEmulatorNull() && mEmulator.isMouseTrackingActive() && !event.isFromSource(InputDevice.SOURCE_MOUSE) && !isSelectingText() && !scrolledWithFinger) {


Don't do this.

agnostic-apollo · 2022-05-29T16:28:23Z

termux-shared/src/main/java/com/termux/shared/android/AndroidUtils.java

    public static void appendPropertyToMarkdownIfSet(StringBuilder markdownString, String label, Object value) {
        if (value == null) return;
-        if (value instanceof String && (((String) value).isEmpty()) || "REL".equals(value)) return;
+        boolean isString = value instanceof String;


Don't do this.

agnostic-apollo · 2022-05-29T16:28:51Z

termux-shared/src/main/java/com/termux/shared/android/PermissionUtils.java

        for (String permission : permissions) {
            result = ContextCompat.checkSelfPermission(context, permission);
-            if (result != PackageManager.PERMISSION_GRANTED) {
+            boolean permissionGranted = (result == PackageManager.PERMISSION_GRANTED);


Don't do this and below ones.

agnostic-apollo · 2022-05-29T16:32:14Z

termux-shared/src/main/java/com/termux/shared/file/filesystem/FileAttributes.java

        HashSet<FilePermission> perms = new HashSet<>();

-        if ((bits & UnixConstants.S_IRUSR) > 0)
+        boolean canOwnerRead = ((bits & UnixConstants.S_IRUSR) > 0);


Don't do this. Can add comments if you must, although code is pretty clear and is from AOSP.

agnostic-apollo · 2022-05-29T16:33:26Z

termux-shared/src/main/java/com/termux/shared/reflection/ReflectionUtils.java

-        public boolean success;
-        public Object value;
-
+    public static class MethodInvokeResult extends InvokeResult{


There must be a space before {.

agnostic-apollo · 2022-05-29T16:34:04Z

termux-shared/src/main/java/com/termux/shared/shell/ShellUtils.java

 public class ShellUtils {

    public static int getPid(Process p) {
+        int invalid = -1;


Don't do this.

agnostic-apollo · 2022-05-29T16:35:28Z

termux-shared/src/main/java/com/termux/shared/termux/crash/TermuxCrashUtils.java


        // If an uncaught exception, then do not notify since the termux app itself would be crashing
-        if (TYPE.UNCAUGHT_EXCEPTION.equals(mType) && TermuxConstants.TERMUX_PACKAGE_NAME.equals(currentPackageName))
+        boolean isUncaughtException = TYPE.UNCAUGHT_EXCEPTION.equals(mType);


Don't do this.

agnostic-apollo · 2022-05-29T16:35:50Z

termux-shared/src/main/java/com/termux/shared/termux/crash/TermuxCrashUtils.java

        sendCrashReportNotification(context, logTag, null, null, reportString, false, false, null, false);
    }

-


Don't remove these functions. These are library utility functions for callers.

agnostic-apollo · 2022-05-29T16:37:27Z

termux-shared/src/main/java/com/termux/shared/termux/file/TermuxFileUtils.java


    private static final String LOG_TAG = "TermuxFileUtils";

+    private static boolean isStringHasValue(String str) {


Skip this file.

agnostic-apollo · 2022-05-29T16:38:02Z

termux-shared/src/main/java/com/termux/shared/termux/interact/TextInputDialogUtils.java


 public final class TextInputDialogUtils {

+    public static final int PADDING_PER_DIP_AT_TOPANDSIDES = 16;


Skip this file.

agnostic-apollo · 2022-05-29T16:38:26Z

termux-shared/src/main/java/com/termux/shared/termux/notification/TermuxNotificationUtils.java


        int nextNotificationId = lastNotificationId + 1;
-        while(nextNotificationId == TermuxConstants.TERMUX_APP_NOTIFICATION_ID || nextNotificationId == TermuxConstants.TERMUX_RUN_COMMAND_NOTIFICATION_ID) {
+


Skip this file.

agnostic-apollo · 2022-05-29T16:38:45Z

termux-shared/src/main/java/com/termux/shared/termux/plugins/TermuxPluginUtils.java

            resultConfig.resultFileBasename = ShellUtils.getExecutableBasename(executionCommand.executable) + "-" + AndroidUtils.getCurrentMilliSecondLocalTimeStamp() + ".log";
    }

-


Skip this file.

agnostic-apollo · 2022-05-29T16:39:38Z

termux-shared/src/main/java/com/termux/shared/termux/shell/am/TermuxAmSocketServer.java

-        TermuxPluginUtils.sendPluginCommandErrorNotification(context, LOG_TAG,
-            localSocketRunConfig.getTitle() + " Socket Server Error", error.getMinimalErrorString(),
-            LocalSocketManager.getErrorMarkdownString(error, localSocketRunConfig, clientSocket));
+


Skip this file.

agnostic-apollo · 2022-05-29T16:40:37Z

termux-shared/src/main/java/com/termux/shared/theme/ThemeUtils.java

     * mode is set to {@link NightMode#SYSTEM} and night mode is enabled by system. */
    public static boolean shouldEnableDarkTheme(Context context, String name) {
-        if (NightMode.TRUE.getName().equals(name))
+        boolean isNightMode = (NightMode.TRUE.getName().equals(name));


Skip this file.

agnostic-apollo · 2022-05-29T16:41:53Z

termux-shared/src/main/java/com/termux/shared/theme/ThemeUtils.java

    /** Get {@link #ATTR_TEXT_COLOR_PRIMARY} value being used by current theme. */
    public static int getTextColorPrimary(Context context) {
-        return getSystemAttrColor(context, ATTR_TEXT_COLOR_PRIMARY);
+        return getSystemAttrColor(context, ATTR_TEXT_COLOR_PRIMARY, 0);


Intermediate library utility functions are often used that pass specific/default values.

agnostic-apollo · 2022-05-29T16:42:36Z

termux-shared/src/main/java/com/termux/shared/view/ViewUtils.java

                ", displayOrientation=" + displayOrientation);
        }

+        boolean isStatusBarHeightTop = (statusBarHeight == windowRect.top);


Skip this file. Only 2 statements will ever exist, so case not worthwhile.

agnostic-apollo · 2022-05-29T16:43:39Z

termux-shared/src/main/java/com/termux/shared/view/ViewUtils.java

-        return r1.left < r1.right && r1.top < r1.bottom
-            // now check if above
-            && r1.left <= r2.left && r1.bottom >= r2.bottom;
+


Skip this too.

agnostic-apollo · 2022-05-29T16:44:44Z

termux-shared/src/main/java/com/termux/shared/termux/extrakeys/ExtraKeysInfo.java


-                JSONObject jobject = normalizeKeyConfig(key);
-
+                JSONObject jsonObject = normalizeKeyConfig(key);


Skip this file too. Rename might be fine, not the if to boolean.

rubinstory and others added 30 commits May 12, 2022 15:06

test

5bd9477

test: git test with change readme

7adb5b9

Merge pull request #1 from BUYA-GH/master

c220eee

test: git test with change readme

Refactor: Extract method isEmulatorNull

eb25e2c

Added|Fixed: Extract Superclass from FieldInvokeResult and

b90cc81

MethodInvokeResult Since FieldInvokeResult and MethodInvokeResult class has a lot in common, extracting them as superclass seems to be more useful for adding other "InvokeResult" classes in the future.

Merge pull request #2 from rubinstory/refactor/ReflectionUtils.java

943a347

Added|Fixed: Extract Superclass from FieldInvokeResult and

Changed: Refactor function name intable to isInTable

93d3efe

Considering a boolean return type and CamelCase style, changed the function name to isInTable.

Changed: Extracting boolean variables in function width()

a55f1fc

According to the code point, the cases where the width is 0 are defined. For clarity, Renamed function variable `ucs` to `codePoint`.

Merge pull request #3 from rubinstory/refactor/ReflectionUtils.java

e9a85b8

refactor: Extract methods from getDeviceInfoMarkdownString()

Merge pull request #1 from cyj893/refactor/WcWidth

cf20e57

Refactor/wc width

refactor: Add Explaining Variables at appendPropertyToMarkdownIfSet()

51dd6b9

Add isString, isEmpty, isREL boolean variables to the method. Now we can check the if statement's condition more easily.

Merge pull request #5 from rubinstory/refactor/AndroidUtils.java

6523497

refactor: Add Explaining Variables at appendPropertyToMarkdownIfSet()

Merge pull request #4 from cyj893/master

e92e0a0

Changed: Refator `WCWidth.java`

Changed: Refactor function name intable to isInTable

888392c

Considering a boolean return type and CamelCase style, changed the function name to isInTable.

Changed: Extracting boolean variables in function width()

d28ccaa

According to the code point, the cases where the width is 0 are defined. For clarity, Renamed function variable `ucs` to `codePoint`.

Changed: Refactor if statement condition with explaining variables

2693133

result != PERMISSION_GRANTED -> boolean permissionGranted context instanceof App..Activity -> boolean isApp..Activity context instanceof Activity -> booelean isActivity

Changed: Replace Error Code with Exception

4795f45

Replaced Error Codes 0 with IllegalArgumentException by each case.

Refactor: Extract methods from setupBootstrapIfNeeded

815b8ae

Extract hasBootstrapError() from setupBootstrapIfNeeded() due to the duplicate code of the method

Refactor: Extract Control down + num variable to static final

dbf19ae

Merge pull request #1 from BUYA-GH/refactor/TerminalView

818811f

Refactor/terminal view

Changed: Extract method getComponentLength() and parseRGB() from …

c5696a5

…`parse()` Include only the parts that make sense for the try-catch.

Merge pull request #8 from BUYA-GH/master

5eedc52

refactor: extract variable and extract method from onStartCommand()

4bb0b2f

The code has two methods and variable name isn't good. so it needs to refactor codes. `getAction`, `chooseAction` are extracted. The code is figured out easily to rename variables

Merge pull request #9 from eyi-jin/refactor/TermuxInstaller.java

7512195

Merge pull request #10 from taehoonkange/refactor/TermuxService.java

a3777e3

Refactor: make "" and " " to String blank and blankSpace for more rea…

c38dfe8

…dable

Refactor: extract function addStringOnMenu at setActionModeCallBacks(…

581ce34

…) for more readable

Changed: Exception handling in each usage.

a324e95

In `tryParseColor()` from `TerminalColors.java` and `updateWith()` from `TerminalColorScheme.java`, setting parsed color is wrapped by try-catch. In `TerminalTest.java`, wrong input testing code is wrapped by try-catch.

Merge pull request #2 from cyj893/refactor/TerminalColors

4e576cf

Refactor/terminal colors

taehoonkange and others added 6 commits May 26, 2022 19:01

Added: Add configureTermuxPreference.java for `RootPreferencesFragm…

559e485

…ent.java` `configureTermuxAPIPreference()`, `configureTermuxFloatPreference()`, etc are very similar method. so Using Template method reduces overlapped codes. At first, make abstract class

Merge pull request #8 from cyj893/pattern/TerminalEmulator

786501a

Changed: Apply State pattern in `TerminalEmulator`

Added: extract class configureTermuxAPIPreference()

d2112d3

Apply Template method for `configureTermuxPreference` and delete orginal code about `configureTermuxAPIPreference()`

Merge pull request #69 from taehoonkange/pattern/RootPreferencesFragm…

0bb9652

…ent.java Added: Add `configureTermuxPreference.java` for `RootPreferencesFragment.java`

Merge pull request #70 from cyj893/master

27b5674

Changed: Apply State pattern in `TerminalEmulator`

Merge pull request #71 from taehoonkange/pattern/configureTermuxAPIPr…

62a06e4

…eference.java Added: extract class `configureTermuxAPIPreference()`

rubinstory closed this May 26, 2022

agnostic-apollo reviewed May 29, 2022

View reviewed changes

		sendCrashReportNotification(context, logTag, null, null, reportString, false, false, null, false);
		}


		private static final String LOG_TAG = "TermuxFileUtils";

		private static boolean isStringHasValue(String str) {


		public final class TextInputDialogUtils {

		public static final int PADDING_PER_DIP_AT_TOPANDSIDES = 16;


		int nextNotificationId = lastNotificationId + 1;
		while(nextNotificationId == TermuxConstants.TERMUX_APP_NOTIFICATION_ID \|\| nextNotificationId == TermuxConstants.TERMUX_RUN_COMMAND_NOTIFICATION_ID) {

		resultConfig.resultFileBasename = ShellUtils.getExecutableBasename(executionCommand.executable) + "-" + AndroidUtils.getCurrentMilliSecondLocalTimeStamp() + ".log";
		}


		JSONObject jobject = normalizeKeyConfig(key);

		JSONObject jsonObject = normalizeKeyConfig(key);

Uh oh!

Refactoring Overall files #2786

Refactoring Overall files #2786

Uh oh!

Conversation

rubinstory commented May 22, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agnostic-apollo commented May 29, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agnostic-apollo May 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agnostic-apollo May 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agnostic-apollo May 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

agnostic-apollo May 29, 2022 •

edited

Loading

agnostic-apollo May 29, 2022 •

edited

Loading

agnostic-apollo May 29, 2022 •

edited

Loading