Skip to content

Comments

generics.hw1 done#4

Open
lbrdev wants to merge 2 commits intoChangeRequest:masterfrom
lbrdev:master
Open

generics.hw1 done#4
lbrdev wants to merge 2 commits intoChangeRequest:masterfrom
lbrdev:master

Conversation

@lbrdev
Copy link

@lbrdev lbrdev commented Feb 21, 2017

No description provided.

@didva didva self-assigned this Feb 22, 2017
package school.lemon.changerequest.java.generics.Container;


public class Container<T> implements ContainerInterface {
Copy link
Contributor

Choose a reason for hiding this comment

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

The thing is, that you should implement generic interface, and it should looks like:
Container<T> implements ContainerInterface<T>
This will help you avoid casting to T.

Copy link
Author

Choose a reason for hiding this comment

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

Anyway its require to cast

return arrayDouble;
}

public Double calcSumForDouble(Double[] array) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need this method?

Copy link
Author

Choose a reason for hiding this comment

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

It was a line what I missed to delete)


public static <K, V> boolean equals(Pair<K, V> pair1, Pair<K, V> pair2) {
boolean flag = false;
if (pair1.getKey().equals(pair2.getKey()) && pair1.getValue().equals(pair2.getValue())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It can be much simpler:
return pair1.getKey().equals(pair2.getKey()) && pair1.getValue().equals(pair2.getValue());

Copy link
Author

Choose a reason for hiding this comment

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

Agree, did

public static <K, V> boolean containsUniqueObjects(Pair<K, V>[] pairs) {
for (Pair<K, V> pair : pairs) {
int i = 1;
while (i < pairs.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not working as expected.
It will always return false, because you are comparing pairs[1] with pairs[1].

public void generateAndPrint() {
T[] arrayNumbers = numberGenerator.generateNumbers();
if (numberGenerator instanceof IntegerGenerator) {
sum = (SumCalculator<T>) new IntegerSumCalculator();
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. It's better to move creation of SumCalculator to constructor.
  2. It's mot flexible, because there is no way to add new type of number generator.
    I'd prefer to create additional method in NumberGenerator that will return specific SumCalculator.


public abstract T[] generateNumbers();

abstract SumCalculator getSumCalculator();
Copy link
Contributor

Choose a reason for hiding this comment

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

protected or public is preferred.

package school.lemon.changerequest.java.generics.Generator;

public class Printer<T extends Number> {
NumberGenerator<T> numberGenerator;
Copy link
Contributor

Choose a reason for hiding this comment

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

Make these fields private


public Printer(NumberGenerator<T> numberGenerator) {
this.numberGenerator = numberGenerator;
if (numberGenerator instanceof IntegerGenerator) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should receive it from generator.


public void generateAndPrint() {
T[] arrayNumbers = numberGenerator.generateNumbers();
numberGenerator.getSumCalculator();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you call this method here? Move it to constructor.

}

public static <K, V> boolean containsUniqueObjects(Pair<K, V>[] pairs) {
for (int a = 0; a < pairs.length; a++ ){
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Please format your code.
  2. General names for loop variables are: i, j, k... Or just some other meaningful name like outerCounter, innerCounter. But not a.
  3. It's better to use a < pairs.lenth - 1, but it's ok to do it like you.

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