-
Notifications
You must be signed in to change notification settings - Fork 7
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
Completed without junit testing #1
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Mar-Alex yes, please write your own unit tests for your code. It would be better to cover everything with your own tests.
As for the abstract implementation — fell free to create additional abstraction if you feel, that it'll produce better results.
@Override | ||
public boolean removeAll(Collection<?> c) { | ||
notNull(c); | ||
boolean result = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This name doesn't show any real context here. I believe something like isChanged
or isModified
could be better here.
private Object[] container; | ||
private int size; | ||
|
||
private int modificationCoef = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is really a coefficient, but probably it is a counter
.
@Override | ||
public boolean retainAll(Collection<?> c) { | ||
notNull(c); | ||
boolean result = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This name doesn't show any real context here. I believe something like isChanged
or isModified
could be better here.
|
||
/**********************************************************************************/ | ||
|
||
private class Itr implements Iterator<E> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to avoid such names, while they do not provide any suitable context within the name.
Probably, something like DirectIterator
or just IteratorImpl
would be better here.
|
||
/**********************************************************************************/ | ||
|
||
private class ListItr extends Itr implements ListIterator<E> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for name here. ListIteratorImpl
or BidirectionalIterator
would be better here.
Should I write junit tests for or functional or just for methods that aren`t covered with standart tests?
Is it necessary to improve my code by creating additional class like
AbstractList
inArrayList
to avoid code duplicating inSubList
?