itachi1706/SingBuses

View on GitHub

Showing 108 of 112 total issues

Define a constant instead of duplicating this literal " WHERE " 5 times.
Open

        String query = "SELECT * FROM " + TABLE_ITEMS + " WHERE " + BUS_STOP_CODE + "=" + busStopCode + ";";

Duplicated string literals make the process of refactoring error-prone, since you must be sure to update all occurrences.

On the other hand, constants can be referenced from many places, but only need to be updated in a single place.

Noncompliant Code Example

With the default threshold of 3:

public void run() {
  prepare("action1");                              // Noncompliant - "action1" is duplicated 3 times
  execute("action1");
  release("action1");
}

@SuppressWarning("all")                            // Compliant - annotations are excluded
private void method1() { /* ... */ }
@SuppressWarning("all")
private void method2() { /* ... */ }

public String method3(String a) {
  System.out.println("'" + a + "'");               // Compliant - literal "'" has less than 5 characters and is excluded
  return "";                                       // Compliant - literal "" has less than 5 characters and is excluded
}

Compliant Solution

private static final String ACTION_1 = "action1";  // Compliant

public void run() {
  prepare(ACTION_1);                               // Compliant
  execute(ACTION_1);
  release(ACTION_1);
}

Exceptions

To prevent generating some false-positives, literals having less than 5 characters are excluded.

Return an empty collection instead of null.
Open

            return null;

Returning null instead of an actual array or collection forces callers of the method to explicitly test for nullity, making them more complex and less readable.

Moreover, in many cases, null is used as a synonym for empty.

Noncompliant Code Example

public static List<Result> getResults() {
  return null;                             // Noncompliant
}

public static Result[] getResults() {
  return null;                             // Noncompliant
}

public static void main(String[] args) {
  Result[] results = getResults();

  if (results != null) {                   // Nullity test required to prevent NPE
    for (Result result: results) {
      /* ... */
    }
  }
}

Compliant Solution

public static List<Result> getResults() {
  return Collections.emptyList();          // Compliant
}

public static Result[] getResults() {
  return new Result[0];
}

public static void main(String[] args) {
  for (Result result: getResults()) {
    /* ... */
  }
}

See

  • CERT, MSC19-C. - For functions that return an array, prefer returning an empty array over a null value
  • CERT, MET55-J. - Return an empty array or collection instead of a null value for methods that return an array or collection

Define a constant instead of duplicating this literal "ETA: " 3 times.
Open

                    .snippet("ETA: " + processArrival(arr1) + " (" + processType(type1) + ")")

Duplicated string literals make the process of refactoring error-prone, since you must be sure to update all occurrences.

On the other hand, constants can be referenced from many places, but only need to be updated in a single place.

Noncompliant Code Example

With the default threshold of 3:

public void run() {
  prepare("action1");                              // Noncompliant - "action1" is duplicated 3 times
  execute("action1");
  release("action1");
}

@SuppressWarning("all")                            // Compliant - annotations are excluded
private void method1() { /* ... */ }
@SuppressWarning("all")
private void method2() { /* ... */ }

public String method3(String a) {
  System.out.println("'" + a + "'");               // Compliant - literal "'" has less than 5 characters and is excluded
  return "";                                       // Compliant - literal "" has less than 5 characters and is excluded
}

Compliant Solution

private static final String ACTION_1 = "action1";  // Compliant

public void run() {
  prepare(ACTION_1);                               // Compliant
  execute(ACTION_1);
  release(ACTION_1);
}

Exceptions

To prevent generating some false-positives, literals having less than 5 characters are excluded.

A "NullPointerException" could be thrown; "buses" is nullable here.
Open

        buses.setLayoutManager(linearLayoutManager);

A reference to null should never be dereferenced/accessed. Doing so will cause a NullPointerException to be thrown. At best, such an exception will cause abrupt program termination. At worst, it could expose debugging information that would be useful to an attacker, or it could allow an attacker to bypass security measures.

Note that when they are present, this rule takes advantage of @CheckForNull and @Nonnull annotations defined in JSR-305 to understand which values are and are not nullable except when @Nonnull is used on the parameter to equals, which by contract should always work with null.

Noncompliant Code Example

@CheckForNull
String getName(){...}

public boolean isNameEmpty() {
  return getName().length() == 0; // Noncompliant; the result of getName() could be null, but isn't null-checked
}
Connection conn = null;
Statement stmt = null;
try{
  conn = DriverManager.getConnection(DB_URL,USER,PASS);
  stmt = conn.createStatement();
  // ...

}catch(Exception e){
  e.printStackTrace();
}finally{
  stmt.close();   // Noncompliant; stmt could be null if an exception was thrown in the try{} block
  conn.close();  // Noncompliant; conn could be null if an exception was thrown
}
private void merge(@Nonnull Color firstColor, @Nonnull Color secondColor){...}

public  void append(@CheckForNull Color color) {
    merge(currentColor, color);  // Noncompliant; color should be null-checked because merge(...) doesn't accept nullable parameters
}
void paint(Color color) {
  if(color == null) {
    System.out.println("Unable to apply color " + color.toString());  // Noncompliant; NullPointerException will be thrown
    return;
  }
  ...
}

See

Rename "sp" which hides the field declared at line 49.
Open

        SharedPreferences sp = PreferenceManager.getDefaultSharedPreferences(this);

Overriding or shadowing a variable declared in an outer scope can strongly impact the readability, and therefore the maintainability, of a piece of code. Further, it could lead maintainers to introduce bugs because they think they're using one variable but are really using another.

Noncompliant Code Example

class Foo {
  public int myField;

  public void doSomething() {
    int myField = 0;
    ...
  }
}

See

Define a constant instead of duplicating this literal "SELECT * FROM " 7 times.
Open

        String query = "SELECT * FROM " + TABLE_ITEMS;

Duplicated string literals make the process of refactoring error-prone, since you must be sure to update all occurrences.

On the other hand, constants can be referenced from many places, but only need to be updated in a single place.

Noncompliant Code Example

With the default threshold of 3:

public void run() {
  prepare("action1");                              // Noncompliant - "action1" is duplicated 3 times
  execute("action1");
  release("action1");
}

@SuppressWarning("all")                            // Compliant - annotations are excluded
private void method1() { /* ... */ }
@SuppressWarning("all")
private void method2() { /* ... */ }

public String method3(String a) {
  System.out.println("'" + a + "'");               // Compliant - literal "'" has less than 5 characters and is excluded
  return "";                                       // Compliant - literal "" has less than 5 characters and is excluded
}

Compliant Solution

private static final String ACTION_1 = "action1";  // Compliant

public void run() {
  prepare(ACTION_1);                               // Compliant
  execute(ACTION_1);
  release(ACTION_1);
}

Exceptions

To prevent generating some false-positives, literals having less than 5 characters are excluded.

Return an empty collection instead of null.
Open

            return null;

Returning null instead of an actual array or collection forces callers of the method to explicitly test for nullity, making them more complex and less readable.

Moreover, in many cases, null is used as a synonym for empty.

Noncompliant Code Example

public static List<Result> getResults() {
  return null;                             // Noncompliant
}

public static Result[] getResults() {
  return null;                             // Noncompliant
}

public static void main(String[] args) {
  Result[] results = getResults();

  if (results != null) {                   // Nullity test required to prevent NPE
    for (Result result: results) {
      /* ... */
    }
  }
}

Compliant Solution

public static List<Result> getResults() {
  return Collections.emptyList();          // Compliant
}

public static Result[] getResults() {
  return new Result[0];
}

public static void main(String[] args) {
  for (Result result: getResults()) {
    /* ... */
  }
}

See

  • CERT, MSC19-C. - For functions that return an array, prefer returning an empty array over a null value
  • CERT, MET55-J. - Return an empty array or collection instead of a null value for methods that return an array or collection

This block of commented-out lines of code should be removed.
Open

        /*switch (sp.getString("companionDevice", "none")) {

Programmers should not comment out code as it bloats programs and reduces readability.

Unused code should be deleted and can be retrieved from source control history if required.

Severity
Category
Status
Source
Language