Skip to content

Use wrapper methods to remove duplicated code #116

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.

Sign up for GitHub

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

Closed
amorellgarcia wants to merge 6 commits into from
Closed

Use wrapper methods to remove duplicated code #116

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3c08017
-Replace String.equals("") with String.isEmpty().
amorellgarcia Dec 24, 2013
fd0542b
-Move "true" and "false" strings to ArduinoConst.TRUE|FALSE
amorellgarcia Dec 24, 2013
b3fb608
-Create "true" and "false string constants in ArduinoConst
amorellgarcia Dec 24, 2013
f5b64b9
-New empty string constant ("") in ArduinoConst.
amorellgarcia Dec 24, 2013
ec300a4
-Add logError, logWarn and logInfo to class Common.
amorellgarcia Dec 24, 2013
2147c52
Remove unused imports.
amorellgarcia Dec 24, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
-Replace String.equals("") with String.isEmpty(). 
-Fix some File.equals(someString) that always returned false.
  • Loading branch information
@amorellgarcia
amorellgarcia committed Dec 24, 2013
commit 3c0801764358850c5e84cd1416d95c85a1713ffb
4 changes: 1 addition & 3 deletions it.baeyens.arduino.common/src/it/baeyens/arduino/common/Common.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,7 @@ public static String UploadPortPrefix() {
* @see parseInt. After error checking and modifications parseInt is used for the conversion
**/
public static int ToInt(String Number) {
if (Number == null)
return 0;
if (Number.equals(""))
if (Number == null || Number.isEmpty())
return 0;
return Integer.parseInt(Number.trim());
}
Expand Down
2 changes: 1 addition & 1 deletion it.baeyens.arduino.core/src/it/baeyens/arduino/toolchain/ArduinoGnuMakefileGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1265,7 +1265,7 @@ private StringBuffer addMacros() {
List<String> subDirList = new ArrayList<String>();
for (IContainer subDir : getSubdirList()) {
IPath projectRelativePath = subDir.getProjectRelativePath();
if (!projectRelativePath.toString().equals("")) //$NON-NLS-1$
if (!projectRelativePath.toString().isEmpty())
subDirList.add(0, projectRelativePath.toString());
}
Collections.sort(subDirList, Collections.reverseOrder());
Expand Down
12 changes: 6 additions & 6 deletions it.baeyens.arduino.core/src/it/baeyens/arduino/tools/ArduinoBoards.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ public String[] getMenuItemNames(String menuLabel, String boardName) {
*
*/
public String[] GetArduinoBoards() {
if (mLastLoadedBoardsFile.equals("")) {
if (mLastLoadedBoardsFile == null || mLastLoadedBoardsFile.toString().isEmpty()) {
String[] sBoards = new String[0];
Copy link
Member

@jantje jantje Dec 24, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is an improvement because (as far as I recall) mLastLoadedBoardsFile is initialized as "" and never set to null.

Copy link
Contributor Author

@amorellgarcia amorellgarcia Dec 24, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is initialized with null at the beginning of class. Seems that some time ago was initialized with "" since there is a line of code commented.

Copy link
Member

@jantje jantje Dec 24, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally hate these double tests (first not null and then test)
I guess it is a style thing.
I do agre that isempty is far better than equals("")

return sBoards;
}
Expand Down Expand Up @@ -247,11 +247,11 @@ public String[] GetArduinoBoards() {
* @author jan
*/
public boolean LoadBoardsFile(String boardsFile) {

if ((mLastLoadedBoardsFile != null) && (mLastLoadedBoardsFile.equals(boardsFile)))
return true; // do nothing when value didn't change
mLastLoadedBoardsFile = new File(boardsFile);
return LoadBoardsFile();
File newFile = new File(boardsFile);
Copy link
Member

@jantje jantje Dec 24, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

funny: now I read (mLastLoadedBoardsFile != null) so there is definitely room for more consistency

Copy link
Contributor Author

@amorellgarcia amorellgarcia Dec 24, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The (mLastLoadedBoardsFile != null) condition is already done in newFile.equals(...).
And the mLastLoadedBoardsFile.equals(boardsFile) condition always fails because compares a File with a String. Seeing source code of OpenJDK, that method has the following implementation:

    public boolean equals(Object obj) {
        if ((obj != null) && (obj instanceof File)) {
            return compareTo((File)obj) == 0;
        }
        return false;
    }

So maybe this fixes a bug that forced always calling to LoadBoardsFile().
What do you think?

Copy link
Member

@jantje jantje Dec 24, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think debugging is the only way to find out for sure.

if (newFile.equals(mLastLoadedBoardsFile))
return true; // do nothing when value didn't change
mLastLoadedBoardsFile = newFile;
return LoadBoardsFile();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion it.baeyens.arduino.core/src/it/baeyens/arduino/tools/ArduinoHelpers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ public static void addArduinoCodeToProject(IProject project, ICConfigurationDesc
String buildCoreFolder = getBuildEnvironmentVariable(configurationDescription, ENV_KEY_build_core_folder, "");
addCodeFolder(project, PATH_VARIABLE_NAME_ARDUINO_PLATFORM, ARDUINO_CORE_FOLDER_NAME + "/" + buildCoreFolder, "arduino/core",
configurationDescription);
if (!boardVariant.equals("")) // this is Arduino version 1.0
if (!boardVariant.isEmpty()) // this is Arduino version 1.0
{
ArduinoHelpers.addCodeFolder(project, PATH_VARIABLE_NAME_ARDUINO_PINS, boardVariant, "arduino/variant", configurationDescription);
} else {
Expand Down
2 changes: 1 addition & 1 deletion it.baeyens.arduino.core/src/it/baeyens/arduino/ui/ArduinoSelectionPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ protected void validatePage() {
.getItemCount() == 0);
}

mValidAndComplete = !mcontrolBoardName.getText().trim().equals("") && !controlUploadPort.getText().trim().equals("")
mValidAndComplete = !mcontrolBoardName.getText().trim().isEmpty() && !controlUploadPort.getText().trim().isEmpty()
&& MenuOpionsValidAndComplete;
feedbackControl.setText(mValidAndComplete ? "true" : "false");
if (mValidAndComplete)
Expand Down
2 changes: 1 addition & 1 deletion it.baeyens.arduino.core/src/it/baeyens/arduino/ui/Import_Source_Folder_Page.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ protected ITreeContentProvider getFolderProvider() {
}

public boolean canFinish() {
return !((controlLibraryPath.getText().equals("")) || (getContainerFullPath() == null));
return !(controlLibraryPath.getText().isEmpty() || getContainerFullPath() == null);
}

public String GetLibraryFolder() {
Expand Down