Skip to content

Conversation

@kravtsun
Copy link

@kravtsun kravtsun commented Apr 26, 2017

Copy link

@sproshev sproshev left a comment

Choose a reason for hiding this comment

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

надо исправлять, это задание не такое сложное, как оно написано

// Список треков, отсортированный лексикографически по названию, включающий все треки альбомов из 'albums'
public static List<String> allTracksSorted(Stream<Album> albums) {
throw new UnsupportedOperationException();
Stream<Album> sortedAlbums = albums.sorted(Comparator.comparing(Album::getName));
Copy link

@sproshev sproshev May 5, 2017

Choose a reason for hiding this comment

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

зачем эта сортировка? -0.25

(Album album) -> album.getTracks().stream().map(track -> {
return track.getName();
});
return sortedAlbums.flatMap(getTracksOfAlbum).sorted(String::compareTo).collect(Collectors.toList());
Copy link

Choose a reason for hiding this comment

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

т.к. строки наследуются от Comparable, то в sorted ничего передавать не надо

(Album album) -> album.getTracks().stream().map(track -> {
return track.getName();
});
return sortedAlbums.flatMap(getTracksOfAlbum).sorted(String::compareTo).collect(Collectors.toList());
Copy link

Choose a reason for hiding this comment

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

этот flatMap может быть представлен в виде цепочки flatMap/map, т.ч. никаких переменных не потребуется

в целом это применимо ко всем методам в этом классе

Collector.of(listSupplier, listAccumulator, listStringCombiner);
Collector<Album, ?, Map<Artist, List<String>>> groupingCollector =
Collectors.groupingBy(a -> a.getArtist(), albumToStringList);
return albums.collect(groupingCollector);
Copy link

Choose a reason for hiding this comment

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

чересчур, достаточно скомбинировать Collectors.groupingBy, Collectors.mapping и Collectors.toList

public static long countAlbumDuplicates(Stream<Album> albums) {
throw new UnsupportedOperationException();
Collector<Album, ?, Map<Album, Integer>> counter =
Collectors.groupingBy(a -> a, Collectors.summingInt(a -> 1));
Copy link

Choose a reason for hiding this comment

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

Collectors.counting()

String name = stringListEntry.getKey();
Integer length = stringListEntry.getValue().stream()
.mapToInt(String::length)
.reduce(0, (l, r) -> l + r);
Copy link

Choose a reason for hiding this comment

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

sum()?

Integer length = stringListEntry.getValue().stream()
.mapToInt(String::length)
.reduce(0, (l, r) -> l + r);
return new Pair(name, length);
Copy link

Choose a reason for hiding this comment

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

new Pair<>(...), warning же светится

, "src/main/java/ru/spbau/mit/FirstPartTasks.java"
, "src/main/java/ru/spbau/mit/SecondPartTasks.java"
);
assertEquals(Arrays.asList(), findQuotes(files, (CharSequence) "qaef2wefadsf"));
Copy link

@sproshev sproshev May 5, 2017

Choose a reason for hiding this comment

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

Collections.emptyList(), уже говорил про него -0.25

, "src/main/java/ru/spbau/mit/SecondPartTasks.java"
);
assertEquals(Arrays.asList(), findQuotes(files, (CharSequence) "qaef2wefadsf"));
List<String> v = findQuotes(files, (CharSequence) "String name");
Copy link

Choose a reason for hiding this comment

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

unused

, " private final String name;"
, " Track(String name, int rating) {"
, " String name = stringListEntry.getKey();")
, findQuotes(files, (CharSequence) "String name"));
Copy link

Choose a reason for hiding this comment

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

к чему здесь постоянный каст к CharSequence?

Copy link

@sproshev sproshev left a comment

Choose a reason for hiding this comment

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

9.5

Function.identity(),
Collectors.counting()
));
Stream<Long> sameCounts = albumsCount.entrySet().stream().map(e -> e.getValue() - 1);
Copy link

Choose a reason for hiding this comment

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

можно здесь сделать mapToInt в конце, потом остается только sum вызвать

Copy link
Author

Choose a reason for hiding this comment

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

Т.е. это common guideline, что в Java в стримах лучше всё в одну строку делать? Вроде же не интерпретируемый язык.

Copy link

Choose a reason for hiding this comment

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

как это связано?)
в одну строку может быть и не стоит (т.к. если цепочка преобразований достаточно длинная, то читаемость заметно падает), но в одно выражение весьма удобно это оформлять.

list.stream().map(...).filter(...).map(...).mapToInt(...) // одна строка и одно выражение

list
  .stream()
  .map(...)
  .filter(...)
  .map(...)
  .mapToInt(...) // несколько строк, но до сих пор одно выражение, я обычно так пишу

одно выражение удобно читать, т.к. последовательность вызовов соответствует тому, что будет происходить в действительности, поэтому код читается за один проход)

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