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

[NEMO-429] SWPP TEAM12 Code Smell Fix #274

Merged
merged 20 commits into from Jan 7, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -34,7 +34,7 @@
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.function.IntPredicate;
import java.util.function.IntPredicate; //NOSONAR
Copy link
Member

Choose a reason for hiding this comment

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

Let's revert this change on the import line. This isn't applicable. Instead, let's make the terminationCondition variable transient on line 59. You can declare it as private transient IntPredicate... there and it will solve the issue 👍


/**
* IRVertex that contains a partial DAG that is iterative.
Expand Down
4 changes: 2 additions & 2 deletions common/src/test/java/org/apache/nemo/common/DAGTest.java
Expand Up @@ -106,8 +106,8 @@ public void testNormalDAG() {
final List<IntegerVertex> topologicalOrder = dag.getTopologicalSort();
assertEquals(topologicalOrder.get(0).getValue(), 4);
assertEquals(topologicalOrder.get(1).getValue(), 5);
assertEquals(topologicalOrder.get(2).getValue(), 1);
assertEquals(topologicalOrder.get(3).getValue(), 2);
assertEquals(1, topologicalOrder.get(2).getValue());
assertEquals(2, topologicalOrder.get(3).getValue());
assertEquals(topologicalOrder.get(4).getValue(), 3);

assertEquals(dag.getRootVertices().size(), 2);
Expand Down
Expand Up @@ -36,9 +36,9 @@
*
* @param <I> input type.
*/
public final class HDFSTextFileTransform<I> extends NoWatermarkEmitTransform<I, String> {
public final class HDFSTextFileTransform<I> extends NoWatermarkEmitTransform<I, String> { //NOSONAR
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove this NOSONAR comment? 😄

private final String path;
private Path fileName;
private Path fileName; //NOSONAR
private List<I> elements;
Copy link
Member

Choose a reason for hiding this comment

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

Please revert the change made on line 39, and make the variables declared on line 41 and 42 transient to remove the code smell. You can declare private transient ... to make the changes. Thanks!


/**
Expand Down
Expand Up @@ -58,9 +58,8 @@ public IRDAG apply(final IRDAG dag) {
// TODO #210: Data-aware dynamic optimization at run-time
dag.topologicalDo(v -> {
// Incoming shuffle edges grouped by the AdditionalOutputTagProperty.
final Function<IREdge, String> groupingFunction = irEdge -> {
return irEdge.getPropertyValue(AdditionalOutputTagProperty.class).orElse(MAIN_OUTPUT_TAG);
};
final Function<IREdge, String> groupingFunction = irEdge ->
irEdge.getPropertyValue(AdditionalOutputTagProperty.class).orElse(MAIN_OUTPUT_TAG);
final Map<String, Set<IREdge>> shuffleEdgesGroupedByTag = dag.getIncomingEdgesOf(v).stream()
.filter(e -> CommunicationPatternProperty.Value.SHUFFLE
.equals(e.getPropertyValue(CommunicationPatternProperty.class).get()))
Expand All @@ -76,7 +75,7 @@ public IRDAG apply(final IRDAG dag) {
// Insert the vertices
final TriggerVertex trigger = new TriggerVertex<>(SkewHandlingUtil.getMessageGenerator(keyExtractor));
final MessageAggregatorVertex mav =
new MessageAggregatorVertex(() -> new HashMap(), SkewHandlingUtil.getMessageAggregator());
new MessageAggregatorVertex(HashMap::new, SkewHandlingUtil.getMessageAggregator());
dag.insert(trigger, mav, SkewHandlingUtil.getEncoder(representativeEdge),
SkewHandlingUtil.getDecoder(representativeEdge), shuffleEdgeGroup, shuffleEdgeGroup);
}
Expand Down
Expand Up @@ -264,6 +264,9 @@ public void processElement(final ProcessContext c) throws Exception {
* Composite transform that wraps the transforms inside the loop.
* The loop updates the user matrix and the item matrix in each iteration.
*/



Copy link
Member

Choose a reason for hiding this comment

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

It seems like these few lines were added by mistake. Could you revert the changes for this file? Thanks! 😄

public static final class UpdateUserAndItemMatrix
extends LoopCompositeTransform<PCollection<KV<Integer, float[]>>, PCollection<KV<Integer, float[]>>> {
private final Integer numFeatures;
Expand Down
Expand Up @@ -76,7 +76,7 @@ public Block createBlock(final String blockId) {
* @throws BlockWriteException if fail to write.
*/
@Override
public void writeBlock(final Block block) throws BlockWriteException {
public void writeBlock(final Block block) {
if (!(block instanceof FileBlock)) {
throw new BlockWriteException(new Throwable(
this.toString() + "only accept " + FileBlock.class.getName()));
Expand All @@ -94,7 +94,7 @@ public void writeBlock(final Block block) throws BlockWriteException {
* @return whether the block exists or not.
*/
@Override
public boolean deleteBlock(final String blockId) throws BlockFetchException {
public boolean deleteBlock(final String blockId) {
final FileBlock fileBlock = (FileBlock) getBlockMap().remove(blockId);
if (fileBlock == null) {
return false;
Expand Down
Expand Up @@ -55,9 +55,9 @@ class SourceVertexDataFetcher extends DataFetcher {

if (!bounded) {
this.watermarkTriggerService = Executors.newScheduledThreadPool(1);
this.watermarkTriggerService.scheduleAtFixedRate(() -> {
watermarkTriggered = true;
}, WATERMARK_PERIOD, WATERMARK_PERIOD, TimeUnit.MILLISECONDS);
this.watermarkTriggerService.scheduleAtFixedRate(() ->
watermarkTriggered = true
, WATERMARK_PERIOD, WATERMARK_PERIOD, TimeUnit.MILLISECONDS);
Copy link
Member

Choose a reason for hiding this comment

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

I think these lines are causing the checkstyle error. Can you append the comma at the end of line 59, instead of at the beginning of line 60?

} else {
this.watermarkTriggerService = null;
}
Expand Down