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 2 #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
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
52 changes: 52 additions & 0 deletions src/main/java/pl/fracz/mcr/comment/AbstractComment.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package pl.fracz.mcr.comment;

import pl.fracz.mcr.preferences.ApplicationSettings;
import pl.fracz.mcr.source.SourceFile;

import java.text.SimpleDateFormat;
import java.util.Date;

public class AbstractComment {
Copy link
Owner

Choose a reason for hiding this comment

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

Myląca nazwa, może sugerować że klasa jest abstrakcyjna

private final SimpleDateFormat CREATION_TIME_FORMAT = new SimpleDateFormat("HH:mm dd.MM.yyyy");

private final Type type;

private final String author;

protected SourceFile sourceFile;

private Date date;

private int lineNumber;

public AbstractComment(Type type, int lineNumber) {
this.type = type;
this.author = ApplicationSettings.getAuthor();
this.lineNumber = lineNumber;
this.date = new Date(System.currentTimeMillis());
}

public Type getType() {
return type;
}

Copy link
Owner

Choose a reason for hiding this comment

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

Po co zwracać niesformatowaną datę w getterze

public Date getDate() {
return date;
}

public String getDateFormatted() {
return CREATION_TIME_FORMAT.format(date);
}

public int getLineNumber() {
return lineNumber;
}

protected String getAuthor() {
return author;
}

Copy link
Owner

Choose a reason for hiding this comment

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

Enum zadeklarowany na drugim końcu pliku, kiedy Type wykorzystywane jest na początku

public static enum Type {
TEXT, VOICE
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package pl.fracz.mcr.comment;

public class CommentNotAddedException extends Exception {
Copy link
Owner

Choose a reason for hiding this comment

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

???????? Pusta klasa dziedzicząca po exception

}
41 changes: 41 additions & 0 deletions src/main/java/pl/fracz/mcr/comment/Comment.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package pl.fracz.mcr.comment;

import pl.fracz.mcr.source.Line;

import java.io.File;

public class Comment extends AbstractComment {
private String text;

private File file;
Copy link
Owner

Choose a reason for hiding this comment

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

Nazwa powinna mówić po co jest określona zmienna, file niewiele mówi ale mogłoby być wykorzystywane np w iteracji kiedy wynika z kontekstu


public Comment(AbstractComment.Type type, Line line) {
super(type, line.get());
}

Copy link
Owner

Choose a reason for hiding this comment

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

Settery i gettery powielają kod walidacji ze zbliżonymi parametrami

public void setText(String text) {
checkValidType(AbstractComment.Type.TEXT);
this.text = text;
}

public String getText() {
checkValidType(AbstractComment.Type.TEXT);
return text;
}

public void setFile(File file) {
checkValidType(AbstractComment.Type.VOICE);
this.file = file;
}

public File getFile() {
checkValidType(AbstractComment.Type.VOICE);
return file;
}

private void checkValidType(AbstractComment.Type type) {
Copy link
Owner

Choose a reason for hiding this comment

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

Zły layout powinno czytać się od góry w dół, więc ta metoda powinna być przed użyciem

if (type != this.getType()) {
throw new IllegalArgumentException("The " + type + " comment is required to set this attribute");
}
}
}
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;

Copy link
Owner

Choose a reason for hiding this comment

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

Historię można przejrzeć na gicie, bezużyteczny komentarz

/*
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;

Copy link
Owner

Choose a reason for hiding this comment

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

Bezużyteczny komentarz

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

Choose a reason for hiding this comment

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

Analogicznie jak wcześniej jedna stała bezużyteczna, druga złego typu

private static final int TWO = 2;

private final int _lineNumber;

private final String _lineOfCode;

Copy link
Owner

Choose a reason for hiding this comment

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

bezużyteczny komentarz

// holds the line number
private TextView lineNumberView;

private TextView lineContent;

private SourceFile sourceFile;

private List<Comment> comments;

Copy link
Owner

Choose a reason for hiding this comment

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

Za dużo parametrów przekazywanych, można upakować w jeden obiekt albo ograniczyć, przy okazji nasuwa się czy nie rozdzielić klasy na mniejsze skoro ma tyle parametrów przekazywanych, bo łamie srp

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);
}

Copy link
Owner

Choose a reason for hiding this comment

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

Małowymowna nazwa

public int get() {
return _lineNumber;
}

Copy link
Owner

Choose a reason for hiding this comment

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

Niepotrzebny komentarz

/**
* Adds a text comment.
*
* @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 {
Comment voiceComment = new Comment(AbstractComment.Type.VOICE, this);
voiceComment.setFile(recodedFile);
comments.add(voiceComment);
if (comments.size() > 0) {
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.

Niejednolite nazewnictwo, parametr bool

private void addLineContent(boolean syntaxColor){
if (!syntaxColor || !SyntaxHighlighter.canBeHighlighted(syntaxColor))
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() {
sourceFile.markConversation(this);
return getComments().size() > TWO;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package pl.fracz.mcr.source;

import pl.fracz.mcr.comment.CommentNotAddedException;

Copy link
Owner

Choose a reason for hiding this comment

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

???????????? Pusta klasa

public class NoSelectedLineException extends CommentNotAddedException {
}
135 changes: 135 additions & 0 deletions src/main/java/pl/fracz/mcr/source/SourceFile.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
package pl.fracz.mcr.source;

import android.content.Context;
import android.graphics.Color;
import android.view.View;
import pl.fracz.mcr.comment.CommentNotAddedException;
import pl.fracz.mcr.preferences.ApplicationSettings;
import pl.fracz.mcr.syntax.PrettifyHighlighter;
import pl.fracz.mcr.syntax.SyntaxHighlighter;
import pl.fracz.mcr.util.FileUtils;

import java.io.File;
import java.io.IOException;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.StringTokenizer;

public class SourceFile {
private static final SyntaxHighlighter SYNTAX_HIGHLIGHTER = new PrettifyHighlighter();

private static final Color SELECTED_LINE_COLOR = Color.parseColor("#444444");

private final View.OnClickListener lineHighlighter = view -> {
if (selectedLine != null) {
selectedLine.setBackgroundColor(Color.TRANSPARENT);
}
selectedLine = (Line) view;
selectedLine.setBackgroundColor(SELECTED_LINE_COLOR);
};

private final String sourceCode;

private final String identifier;

private final String language;

private Line selectedLine;

private String highlightedSourceCode;

public SourceFile(String sourceCode, String language) {
this.sourceCode = sourceCode;
this.language = language;
this.identifier = calculateSHA1SourceChecksum();
}

private String calculateSHA1SourceChecksum() {
try {
MessageDigest md = MessageDigest.getInstance("SHA1");
byte[] digest = md.digest(sourceCode.getBytes());
StringBuffer sb = new StringBuffer();
for (int i = 0; i < digest.length; i++) {
sb.append(Integer.toString((digest[i] & 0xff) + 0x100, 16).substring(1));
}
return sb.toString();
} catch (NoSuchAlgorithmException e) {
throw new AssertionError();
}
}

public Collection<Line> getLines(Context context) {
StringTokenizer tokenizer = new StringTokenizer(getHighlightedSourceCode(), "\n");
Collection<Line> lines = new ArrayList<Line>(tokenizer.countTokens());
while (tokenizer.hasMoreTokens()) {
Line line = new Line(context, this, lines.size() + 1, tokenizer.nextToken());
line.setOnClickListener(lineHighlighter);
lines.add(line);
}
return lines;
}

private String getHighlightedSourceCode() {
if (highlightedSourceCode == null) {
Copy link
Owner

Choose a reason for hiding this comment

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

sam fakt że może być nullem jest błędny

highlightedSourceCode = highlightSourceCode();
}
return highlightedSourceCode;
}

private String highlightSourceCode() {
String code = replaceTabs();
if (ApplicationSettings.highlightSources()) {
return SYNTAX_HIGHLIGHTER.highlight(code, language);
} else {
return code;
}
}

Copy link
Owner

Choose a reason for hiding this comment

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

Można prościej

private String replaceTabs() {
StringBuilder tabReplacement = new StringBuilder();
for (int i = 0; i < ApplicationSettings.getTabSize(); i++) {
tabReplacement.append(" ");
}
return sourceCode.replace("\t", tabReplacement.toString());
}

public String getIdentifier() {
return identifier;
}

public void addTextComment(String comment) throws CommentNotAddedException {
if (selectedLine == null) {
Copy link
Owner

Choose a reason for hiding this comment

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

Niepotrzebny null

throw new NoSelectedLineException();
}
getSelectedLine().addTextComment(comment);
}

public void addVoiceComment(File recordedFile) throws CommentNotAddedException {
if (selectedLine == null) {
throw new NoSelectedLineException();
}
getSelectedLine().createVoiceComment(recordedFile);
}

public Line getSelectedLine() {
return selectedLine;
}

Copy link
Owner

Choose a reason for hiding this comment

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

Pusta metoda

public void markConversation(Line line) {
// Nothing to do, it's perfect.
}

public static SourceFile createFromString(String sourceCode, String language) {
return new SourceFile(sourceCode, language);
}

Copy link
Owner

Choose a reason for hiding this comment

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

Bezużyteczny komentarz

/**
* Creates source file based on a file reference.
*/
public static SourceFile createFromFile(File sourceFile) throws IOException {
String sourceCode = FileUtils.read(sourceFile);
return createFromString(sourceCode, FileUtils.getExtension(sourceFile.getName()));
Copy link
Owner

Choose a reason for hiding this comment

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

Godline

}
}