Skip to content

Conversation

@valasatava
Copy link
Contributor

bug in calculating CDS length

public static final String CDS = "CDS";


public static int base = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be a member of this class rather than a static?

Copy link
Contributor

@josemduarte josemduarte left a comment

Choose a reason for hiding this comment

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

Looks good thanks! Some minor comments only

import static junitx.framework.ComparableAssert.assertEquals;

/**
* Created by Yana Valasatava on 8/14/17.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use the @author tag here and also write a minimal explanation of what the test does?

import java.util.Arrays;
import java.util.List;

import static junitx.framework.ComparableAssert.assertEquals;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be imported from package org.junit.assert. I usually simply do this:

import static org.junit.Assert.*;

which imports all the assert static methods.

@valasatava
Copy link
Contributor Author

Is there anything else I can improve?

@josemduarte josemduarte merged commit 1bedd9a into biojava:master Aug 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants