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

Review Task 1 #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
117 changes: 117 additions & 0 deletions src/main/java/pl/fracz/mcr/source/Line.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
package pl.fracz.mcr.source;

/*
2013-10-23, fracz, first implementation
2013-10-30, fracz, added syntax highlighting
2014-02-26, fracz, added ability to add voice comment
*/

import android.annotation.SuppressLint;
import android.content.Context;
import android.graphics.Color;
import android.graphics.Typeface;
import android.text.Html;
import android.widget.LinearLayout;
import android.widget.TextView;
import pl.fracz.mcr.comment.AbstractComment;
import pl.fracz.mcr.comment.Comment;
import pl.fracz.mcr.comment.CommentNotAddedException;

import java.io.File;
import java.io.Serializable;
import java.util.List;

/**
* View that represents one line of code.
*/
@SuppressLint("ViewConstructor")
public class Line extends LinearLayout implements Serializable {
private static final long serialVersionUID = 3076583280108678995L;
private static final int TWO = 2;
Copy link
Owner

Choose a reason for hiding this comment

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

Brak spójności nazewnictwa zmiennych prywatnych


private final int _lineNumber;

private final String _lineOfCode;

// holds the line number
private TextView lineNumberView;

private TextView lineContent;

private SourceFile sourceFile;

private List<Comment> comments;

public Line(Context context, SourceFile sourceFile, int lineNumber,
String lineOfCode, boolean syntaxColor) {
super(context);
this.sourceFile = sourceFile;
this._lineNumber = lineNumber;
this._lineOfCode = lineOfCode;
setOrientation(LinearLayout.HORIZONTAL);

lineNumberView = new TextView(getContext());
lineNumberView.setText(String.format("%d.", lineNumber););
lineNumberView.setSingleLine();
lineNumberView.setWidth(30);
addView(lineNumberView);

TextView lineContent = new TextView(getContext());
addLineContent(syntaxColor);
}

public int get() {
Copy link
Owner

Choose a reason for hiding this comment

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

Metoda nie mówi, co właściwie pobieramy

return _lineNumber;
}

/**
* Adds a text comment.
Copy link
Owner

Choose a reason for hiding this comment

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

Nadmiarowy komentarz

*
* @param comment
* @throws CommentNotAddedException
*/
public void addTextComment(String comment) throws CommentNotAddedException {
Comment textComment = new Comment(AbstractComment.Type.TEXT, this);
textComment.setText(comment);
comments.add(textComment);
if (comments.size() > 0) {
lineNumberView.setBackgroundColor(Color.parseColor("#008000"));
}
}

/**
* Adds a voice comment.
*
* @param recordedFile
* @throws CommentNotAddedException
*/
public void createVoiceComment(File recodedFile) throws CommentNotAddedException {
Copy link
Owner

Choose a reason for hiding this comment

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

Inconsistency of naming - tutaj mamy create, wcześniej add.

Comment voiceComment = new Comment(AbstractComment.Type.VOICE, this);
voiceComment.setFile(recodedFile);
comments.add(voiceComment);
if (comments.size() > 0) {
Copy link
Owner

Choose a reason for hiding this comment

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

Duplikacja kodu - może warto wyłuskać nową metodę?

lineNumberView.setBackgroundColor(Color.parseColor("#008000"));
}
}

// public void addVideoComment(File videoFile) throws CommentNotAddedException {
Copy link
Owner

Choose a reason for hiding this comment

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

Pozostałości w komentarzach

// }

private void addLineContent(boolean syntaxColor){
if (!syntaxColor || !SyntaxHighlighter.canBeHighlighted(syntaxColor))
Copy link
Owner

Choose a reason for hiding this comment

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

Warunek w ifie można uczynić bardziej czytelnym przenosząc go to osobnej metody. Dodatkowo mamy "negative condition" oraz metodę z booleanem w argumentach - można zrobić dwa osobne przypadki - addLineContentWithSyntaxColour i druga without syntax.

lineContent.setText(Html.fromHtml(lineOfCode));
else
lineContent.setText(SyntaxHighlighter.highlight(Html.fromHtml(lineOfCode)));
lineContent.setTypeface(Typeface.MONOSPACE);
addView(lineContent);
}

public List<Comment> getComments(){
return this.comments;
}

public boolean hasConversation() {
Copy link
Owner

Choose a reason for hiding this comment

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

Metoda nie tylko zwraca warunek, ale robi też coś pobocznego - "markConversation"

sourceFile.markConversation(this);
return getComments().size() > TWO;
}
}