Skip to content

Commit

Permalink
[5392] Add new rules to PMD - design part 3
Browse files Browse the repository at this point in the history
  • Loading branch information
@kanghj
kanghj committed May 19, 2016
1 parent 582b8b1 commit 07bfcd8
Show file tree
Hide file tree
Showing 51 changed files with 176 additions and 274 deletions.
42 changes: 8 additions & 34 deletions src/client/java/teammates/client/scripts/AdminEmailListGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,30 +289,17 @@ private boolean isInstructorCreatedInRange(Instructor instructor) {
} else if (emailListConfig.instructorCreatedDateRangeStart != null
&& emailListConfig.instructorCreatedDateRangeEnd == null) {
//after a specific date
if (instructorCreatedAt.after(emailListConfig.instructorCreatedDateRangeStart)) {
return true;
} else {
return false;
}

return instructorCreatedAt.after(emailListConfig.instructorCreatedDateRangeStart);
} else if (emailListConfig.instructorCreatedDateRangeStart == null
&& emailListConfig.instructorCreatedDateRangeEnd != null) {
//before a specific date
if (instructorCreatedAt.before(emailListConfig.instructorCreatedDateRangeEnd)) {
return true;
} else {
return false;
}
return instructorCreatedAt.before(emailListConfig.instructorCreatedDateRangeEnd);

} else if (emailListConfig.instructorCreatedDateRangeStart != null
&& emailListConfig.instructorCreatedDateRangeEnd != null) {
//within a date interval
if (instructorCreatedAt.after(emailListConfig.instructorCreatedDateRangeStart)
&& instructorCreatedAt.before(emailListConfig.instructorCreatedDateRangeEnd)) {
return true;
} else {
return false;
}
return instructorCreatedAt.after(emailListConfig.instructorCreatedDateRangeStart)
&& instructorCreatedAt.before(emailListConfig.instructorCreatedDateRangeEnd);
}

return false;
Expand Down Expand Up @@ -358,30 +345,17 @@ private boolean isStudentCreatedInRange(Student student) {
} else if (emailListConfig.studentCreatedDateRangeStart != null
&& emailListConfig.studentCreatedDateRangeEnd == null) {
//after a specific date
if (studentCreatedAt.after(emailListConfig.studentCreatedDateRangeStart)) {
return true;
} else {
return false;
}
return studentCreatedAt.after(emailListConfig.studentCreatedDateRangeStart);

} else if (emailListConfig.studentCreatedDateRangeStart == null
&& emailListConfig.studentCreatedDateRangeEnd != null) {
//before a specific date
if (studentCreatedAt.before(emailListConfig.studentCreatedDateRangeEnd)) {
return true;
} else {
return false;
}

return studentCreatedAt.before(emailListConfig.studentCreatedDateRangeEnd);
} else if (emailListConfig.studentCreatedDateRangeStart != null
&& emailListConfig.studentCreatedDateRangeEnd != null) {
//within a date interval
if (studentCreatedAt.after(emailListConfig.studentCreatedDateRangeStart)
&& studentCreatedAt.before(emailListConfig.studentCreatedDateRangeEnd)) {
return true;
} else {
return false;
}
return studentCreatedAt.after(emailListConfig.studentCreatedDateRangeStart)
&& studentCreatedAt.before(emailListConfig.studentCreatedDateRangeEnd);
}

return false;
Expand Down
2 changes: 1 addition & 1 deletion src/client/java/teammates/client/scripts/PerformanceProfiler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ public class PerformanceProfiler extends Thread {
private DataBundle data;
private Gson gson = Utils.getTeammatesGson();
private Map<String, ArrayList<Float>> results = new HashMap<String, ArrayList<Float>>();
private Browser browser;

public PerformanceProfiler(String path) {
reportFilePath = path;
Expand All @@ -90,6 +89,7 @@ public void run() {
} catch (IOException e1) {
e1.printStackTrace();
}
Browser browser;
for (int i = 0; i < NUM_OF_RUNS; i++)
{
browser = BrowserPool.getBrowser();
Expand Down
20 changes: 8 additions & 12 deletions src/main/java/teammates/common/datatransfer/FeedbackConstantSumQuestionDetails.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,8 @@ public String getQuestionWithExistingResponseSubmissionFormHtml(
"${qnIdx}", Integer.toString(qnIdx),
"${responseIdx}", Integer.toString(responseIdx),
"${constSumOptionVisibility}", distributeToRecipients ? "style=\"display:none\"" : "",
"${constSumToRecipientsValue}", distributeToRecipients ? "true" : "false",
"${constSumPointsPerOptionValue}", pointsPerOption ? "true" : "false",
"${constSumToRecipientsValue}", Boolean.toString(distributeToRecipients),
"${constSumPointsPerOptionValue}", Boolean.toString(pointsPerOption),
"${constSumNumOptionValue}", Integer.toString(constSumOptions.size()),
"${constSumPointsValue}", Integer.toString(points),
"${constSumUnevenDistributionValue}", Boolean.toString(forceUnevenDistribution),
Expand Down Expand Up @@ -244,8 +244,8 @@ public String getQuestionWithoutExistingResponseSubmissionFormHtml(
"${qnIdx}", Integer.toString(qnIdx),
"${responseIdx}", Integer.toString(responseIdx),
"${constSumOptionVisibility}", distributeToRecipients ? "style=\"display:none\"" : "",
"${constSumToRecipientsValue}", distributeToRecipients ? "true" : "false",
"${constSumPointsPerOptionValue}", pointsPerOption ? "true" : "false",
"${constSumToRecipientsValue}", Boolean.toString(distributeToRecipients),
"${constSumPointsPerOptionValue}", Boolean.toString(pointsPerOption),
"${constSumNumOptionValue}", Integer.toString(constSumOptions.size()),
"${constSumPointsValue}", Integer.toString(points),
"${constSumUnevenDistributionValue}", Boolean.toString(forceUnevenDistribution),
Expand Down Expand Up @@ -279,7 +279,7 @@ public String getQuestionSpecificEditFormHtml(int questionNumber) {
"${questionNumber}", Integer.toString(questionNumber),
"${Const.ParamsNames.FEEDBACK_QUESTION_NUMBEROFCHOICECREATED}", Const.ParamsNames.FEEDBACK_QUESTION_NUMBEROFCHOICECREATED,
"${numOfConstSumOptions}", Integer.toString(numOfConstSumOptions),
"${constSumToRecipientsValue}", distributeToRecipients ? "true" : "false",
"${constSumToRecipientsValue}", Boolean.toString(distributeToRecipients),
"${selectedConstSumPointsPerOption}", pointsPerOption ? "selected" : "",
"${constSumOptionTableVisibility}", distributeToRecipients ? "style=\"display:none\"" : "",
"${constSumPoints}", points == 0 ? "100" : Integer.toString(points),
Expand Down Expand Up @@ -526,8 +526,8 @@ public boolean isChangesRequiresResponseDeletion(
FeedbackConstantSumQuestionDetails newConstSumDetails = (FeedbackConstantSumQuestionDetails) newDetails;

if (this.numOfConstSumOptions != newConstSumDetails.numOfConstSumOptions
|| this.constSumOptions.containsAll(newConstSumDetails.constSumOptions) == false
|| newConstSumDetails.constSumOptions.containsAll(this.constSumOptions) == false) {
|| !this.constSumOptions.containsAll(newConstSumDetails.constSumOptions)
|| !newConstSumDetails.constSumOptions.containsAll(this.constSumOptions)) {
return true;
}

Expand All @@ -543,11 +543,7 @@ public boolean isChangesRequiresResponseDeletion(
return true;
}

if (this.forceUnevenDistribution != newConstSumDetails.forceUnevenDistribution) {
return true;
}

return false;
return this.forceUnevenDistribution != newConstSumDetails.forceUnevenDistribution;
}

@Override
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/teammates/common/datatransfer/FeedbackContributionQuestionDetails.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ private String getQuestionResultsStatisticsHtmlQuestionView(List<FeedbackRespons
boolean hideRecipient = false;
FeedbackParticipantType type = question.recipientType;
for (FeedbackResponseAttributes response : responses) {
if (bundle.visibilityTable.get(response.getId())[1] == false
if (!bundle.visibilityTable.get(response.getId())[1]
&& type != FeedbackParticipantType.SELF
&& type != FeedbackParticipantType.NONE) {
hideRecipient = true;
Expand All @@ -272,7 +272,7 @@ private String getQuestionResultsStatisticsHtmlQuestionView(List<FeedbackRespons

String displayName = name;
String displayTeam = team;
if (hideRecipient == true) {
if (hideRecipient) {
String hash = Integer.toString(Math.abs(name.hashCode()));
displayName = type.toSingularFormString();
displayName = "Anonymous " + displayName + " " + hash;
Expand Down Expand Up @@ -345,7 +345,7 @@ public String getQuestionResultStatisticsCsv(

FeedbackParticipantType type = question.recipientType;
for (FeedbackResponseAttributes response : responses) {
if (bundle.visibilityTable.get(response.getId())[1] == false
if (!bundle.visibilityTable.get(response.getId())[1]
&& type != FeedbackParticipantType.SELF
&& type != FeedbackParticipantType.NONE) {
hideRecipient = true;
Expand All @@ -368,7 +368,7 @@ public String getQuestionResultStatisticsCsv(
String displayName = name;
String displayTeam = team;
String displayEmail = email;
if (hideRecipient == true) {
if (hideRecipient) {
String hash = Integer.toString(Math.abs(name.hashCode()));
displayName = type.toSingularFormString();
displayName = "Anonymous " + displayName + " " + hash;
Expand Down Expand Up @@ -721,7 +721,7 @@ public List<String> validateResponseAttributes(
if (frd.getAnswer() == Const.POINTS_NOT_SURE || frd.getAnswer() == Const.POINTS_NOT_SUBMITTED) {
validAnswer = true;
}
if (validAnswer == false) {
if (!validAnswer) {
errors.add(Const.FeedbackQuestion.CONTRIB_ERROR_INVALID_OPTION);
}
}
Expand Down
10 changes: 3 additions & 7 deletions src/main/java/teammates/common/datatransfer/FeedbackMcqQuestionDetails.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,20 +107,16 @@ public boolean isChangesRequiresResponseDeletion(FeedbackQuestionDetails newDeta
FeedbackMcqQuestionDetails newMcqDetails = (FeedbackMcqQuestionDetails) newDetails;

if (this.numOfMcqChoices != newMcqDetails.numOfMcqChoices
|| this.mcqChoices.containsAll(newMcqDetails.mcqChoices) == false
|| newMcqDetails.mcqChoices.containsAll(this.mcqChoices) == false) {
|| !this.mcqChoices.containsAll(newMcqDetails.mcqChoices)
|| !newMcqDetails.mcqChoices.containsAll(this.mcqChoices)) {
return true;
}

if (this.generateOptionsFor != newMcqDetails.generateOptionsFor) {
return true;
}

if (this.otherEnabled != newMcqDetails.otherEnabled) {
return true;
}

return false;
return this.otherEnabled != newMcqDetails.otherEnabled;
}

@Override
Expand Down
10 changes: 3 additions & 7 deletions src/main/java/teammates/common/datatransfer/FeedbackMsqQuestionDetails.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,20 +108,16 @@ public boolean isChangesRequiresResponseDeletion(FeedbackQuestionDetails newDeta
FeedbackMsqQuestionDetails newMsqDetails = (FeedbackMsqQuestionDetails) newDetails;

if (this.numOfMsqChoices != newMsqDetails.numOfMsqChoices
|| this.msqChoices.containsAll(newMsqDetails.msqChoices) == false
|| newMsqDetails.msqChoices.containsAll(this.msqChoices) == false) {
|| !this.msqChoices.containsAll(newMsqDetails.msqChoices)
|| !newMsqDetails.msqChoices.containsAll(this.msqChoices)) {
return true;
}

if (this.generateOptionsFor != newMsqDetails.generateOptionsFor) {
return true;
}

if (this.otherEnabled != newMsqDetails.otherEnabled) {
return true;
}

return false;
return this.otherEnabled != newMsqDetails.otherEnabled;
}

@Override
Expand Down
11 changes: 4 additions & 7 deletions src/main/java/teammates/common/datatransfer/FeedbackNumericalScaleQuestionDetails.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ private List<String> getHiddenRecipients(
List<String> hiddenRecipients = new ArrayList<String>(); // List of recipients to hide
FeedbackParticipantType type = question.recipientType;
for (FeedbackResponseAttributes response : responses) {
if (bundle.visibilityTable.get(response.getId())[1] == false
if (!bundle.visibilityTable.get(response.getId())[1]
&& type != FeedbackParticipantType.SELF
&& type != FeedbackParticipantType.NONE) {

Expand Down Expand Up @@ -658,12 +658,9 @@ public boolean isChangesRequiresResponseDeletion(
FeedbackNumericalScaleQuestionDetails newNumScaleDetails =
(FeedbackNumericalScaleQuestionDetails) newDetails;

if (this.minScale != newNumScaleDetails.minScale
|| this.maxScale != newNumScaleDetails.maxScale
|| this.step != newNumScaleDetails.step) {
return true;
}
return false;
return this.minScale != newNumScaleDetails.minScale
|| this.maxScale != newNumScaleDetails.maxScale
|| this.step != newNumScaleDetails.step;
}

@Override
Expand Down
48 changes: 22 additions & 26 deletions src/main/java/teammates/common/datatransfer/FeedbackQuestionAttributes.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,35 +175,35 @@ public List<String> getVisibilityMessage() {
line += "can see your response";

// Visibility fragment: e.g. can see your name, but not...
if (showRecipientNameTo.contains(participant) == false) {
if (showGiverNameTo.contains(participant) == true) {
if (showRecipientNameTo.contains(participant)) {
if (participant != FeedbackParticipantType.RECEIVER
&& recipientType != FeedbackParticipantType.NONE) {
line += ", the name of the recipient";
}

if (showGiverNameTo.contains(participant)) {
line += ", and your name";
} else {
line += ", but not your name";
}
} else {
if (showGiverNameTo.contains(participant)) {
line += ", and your name";
}

if (recipientType == FeedbackParticipantType.NONE) {
if (showGiverNameTo.contains(participant) == false) {
if (!showGiverNameTo.contains(participant)) {
line += ", but not your name";
}
} else {
line += ", but not the name of the recipient";

if (showGiverNameTo.contains(participant) == false) {
if (!showGiverNameTo.contains(participant)) {
line += ", or your name";
}
}

} else if (showRecipientNameTo.contains(participant) == true) {
if (participant != FeedbackParticipantType.RECEIVER
&& recipientType != FeedbackParticipantType.NONE) {
line += ", the name of the recipient";
}

if (showGiverNameTo.contains(participant)) {
line += ", and your name";
} else {
line += ", but not your name";
}
}
}

line += ".";
message.add(line);
Expand Down Expand Up @@ -251,22 +251,18 @@ public boolean isResponseVisibleTo(FeedbackParticipantType userType) {
* @return
*/
public boolean isChangesRequiresResponseDeletion(FeedbackQuestionAttributes newAttributes) {
if (newAttributes.giverType.equals(this.giverType) == false
|| newAttributes.recipientType.equals(this.recipientType) == false) {
return true;
}

if (this.showResponsesTo.containsAll(newAttributes.showResponsesTo) == false
|| this.showGiverNameTo.containsAll(newAttributes.showGiverNameTo) == false
|| this.showRecipientNameTo.containsAll(newAttributes.showRecipientNameTo) == false) {
if (!newAttributes.giverType.equals(this.giverType)
|| !newAttributes.recipientType.equals(this.recipientType)) {
return true;
}

if (this.getQuestionDetails().isChangesRequiresResponseDeletion(newAttributes.getQuestionDetails())) {
if (!this.showResponsesTo.containsAll(newAttributes.showResponsesTo)
|| !this.showGiverNameTo.containsAll(newAttributes.showGiverNameTo)
|| !this.showRecipientNameTo.containsAll(newAttributes.showRecipientNameTo)) {
return true;
}

return false;
return this.getQuestionDetails().isChangesRequiresResponseDeletion(newAttributes.getQuestionDetails());
}

@Override
Expand Down
8 changes: 2 additions & 6 deletions src/main/java/teammates/common/datatransfer/FeedbackQuestionDetails.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,8 @@ public String getNoResponseTextInHtml(String giverEmail, String recipientEmail,
public boolean shouldShowNoResponseText(String giverEmail, String recipientEmail,
FeedbackQuestionAttributes question) {
// we do not show all possible responses
if (question.recipientType == FeedbackParticipantType.STUDENTS
|| question.recipientType == FeedbackParticipantType.TEAMS) {
return false;
}

return true;
return question.recipientType != FeedbackParticipantType.STUDENTS
&& question.recipientType != FeedbackParticipantType.TEAMS;
}

public String getNoResponseTextInCsv(String giverEmail, String recipientEmail,
Expand Down
8 changes: 2 additions & 6 deletions src/main/java/teammates/common/datatransfer/FeedbackRankOptionsQuestionDetails.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,13 +353,9 @@ private Map<String, List<Integer>> generateOptionRanksMapping(
public boolean isChangesRequiresResponseDeletion(FeedbackQuestionDetails newDetails) {
FeedbackRankOptionsQuestionDetails newRankQuestionDetails = (FeedbackRankOptionsQuestionDetails) newDetails;

if (this.options.size() != newRankQuestionDetails.options.size()
return this.options.size() != newRankQuestionDetails.options.size()
|| !this.options.containsAll(newRankQuestionDetails.options)
|| !newRankQuestionDetails.options.containsAll(this.options)) {
return true;
}

return false;
|| !newRankQuestionDetails.options.containsAll(this.options);
}

@Override
Expand Down
Loading

0 comments on commit 07bfcd8

Please sign in to comment.