Problem
Add a Java Streams skill eval for a real refactoring case where the model is asked to improve an existing Java 25 class that extracts repository-source declarations from Trello card text.
The important lesson is not "always use streams". The eval should teach that when a stream pipeline is used to transform input into output, it should normally produce the result through the pipeline instead of using forEach(...) to mutate an external collection.
This case also teaches the difference between these three approaches for a line-to-declaration transformation:
forEach(...) with external mutation: works, but is not an idiomatic stream-based refactor.
flatMap(...) with Stream.of(...) and Stream.empty(): valid, but creates a tiny stream for every input line.
mapMulti(...): preferred for this case, because each line produces zero or one declaration.
The desired eval outcome is that the model replaces the original eager line splitting and mutable accumulation with a String.lines() plus mapMulti(...) based implementation, not with a side-effecting forEach(...) or a line-level flatMap(...).
Original code provided to the agent
The agent was given the full class below. The relevant methods are declarations(...) and addDeclarations(...), but the full class matters because the refactor must preserve surrounding behavior, including LABELED_SOURCE, Declaration, labelMode(...), and the parsing conflict logic.
Full original class
package ch.fmartin.symphony.trello.repository;
import ch.fmartin.symphony.trello.config.EffectiveConfig;
import ch.fmartin.symphony.trello.domain.Card;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
public final class RepositorySourceResolver {
private static final Pattern LABELED_SOURCE = Pattern.compile(
"(?i)^[\\t ]*(repository[\\t ]+(?:url|path)|repo[\\t ]+(?:url|path)|local[\\t ]+(?:checkout|path)|checkout|repository|repo)[\\t ]*:[\\t ]*(.*)$");
private static final Pattern URI_SCHEME = Pattern.compile("^([A-Za-z][A-Za-z0-9+.-]*):");
private static final Pattern WINDOWS_DRIVE_PATH = Pattern.compile("^[A-Za-z]:[\\\\/].*");
public RepositorySourceSelection select(Card card, EffectiveConfig.RepositoryConfig repository) {
RepositorySourceSelection explicit = explicitSource(card);
if (explicit.status() != RepositorySourceSelection.Status.NONE) {
return explicit;
}
return workflowDefault(repository);
}
private RepositorySourceSelection explicitSource(Card card) {
List<Declaration> declarations = declarations(card);
if (declarations.isEmpty()) {
return RepositorySourceSelection.none();
}
if (declarations.size() == 1) {
Declaration declaration = declarations.getFirst();
return parse(declaration.value(), RepositorySource.Origin.CARD, declaration.mode());
}
List<RepositorySourceSelection> parsed = new ArrayList<>(declarations.size());
for (Declaration declaration : declarations) {
parsed.add(parse(declaration.value(), RepositorySource.Origin.CARD, declaration.mode()));
}
RepositorySourceSelection first = parsed.getFirst();
if (first.status() == RepositorySourceSelection.Status.SELECTED
&& parsed.stream().allMatch(selection -> equivalent(first, selection))) {
return first;
}
return invalid(
"repository_source_conflict",
"Multiple explicit repository source declarations conflict. Keep one source declaration or make every declaration identical.");
}
private RepositorySourceSelection workflowDefault(EffectiveConfig.RepositoryConfig repository) {
if (repository.defaultUrl() != null) {
return parse(
repository.defaultUrl(), RepositorySource.Origin.WORKFLOW_DEFAULT_URL, SourceMode.REMOTE_OR_FILE);
}
if (repository.defaultPath() != null) {
return RepositorySourceSelection.selected(new RepositorySource(
RepositorySource.Kind.LOCAL_PATH,
RepositorySource.Origin.WORKFLOW_DEFAULT_PATH,
repository.defaultPath().toString(),
null,
repository.defaultPath().toAbsolutePath().normalize()));
}
return RepositorySourceSelection.none();
}
private static RepositorySourceSelection parse(String rawValue, RepositorySource.Origin origin, SourceMode mode) {
String value = mode == SourceMode.LOCAL_PATH ? rawValue.strip() : stripTokenPunctuation(rawValue);
if (value.isBlank()) {
return invalid("repository_source_missing", "The selected repository source is blank.");
}
if (!RepositorySourceText.safePromptLine(value)) {
return malformed(mode);
}
if (mode == SourceMode.LOCAL_PATH) {
return localPath(value, origin);
}
if (mode.allowsLocalPath() && windowsDrivePath(value)) {
return localPath(value, origin);
}
if (fileUri(value)) {
return fileUriPath(value, origin);
}
RepositorySourceSelection remote = remote(value, origin);
if (remote.status() != RepositorySourceSelection.Status.NONE) {
return remote;
}
if (mode == SourceMode.REMOTE_ONLY || mode == SourceMode.REMOTE_OR_FILE || explicitUriScheme(value)) {
return invalid(
"repository_remote_unsupported",
"The selected repository remote uses an unsupported transport. Use HTTPS, SSH, SCP-style SSH, or a local checkout path.");
}
return localPath(rawValue.strip(), origin);
}
private static RepositorySourceSelection remote(String value, RepositorySource.Origin origin) {
if (!explicitUriScheme(value)) {
RepositorySourceSelection scp = scpRemote(value, origin);
if (scp.status() != RepositorySourceSelection.Status.NONE) {
return scp;
}
}
try {
URI uri = new URI(value);
String scheme = lower(uri.getScheme());
if ("http".equals(scheme) || "https".equals(scheme)) {
return httpRemote(uri, origin);
}
if ("ssh".equals(scheme)) {
return sshRemote(uri, origin);
}
return RepositorySourceSelection.none();
} catch (URISyntaxException | IllegalArgumentException e) {
return RepositorySourceSelection.none();
}
}
private static RepositorySourceSelection httpRemote(URI uri, RepositorySource.Origin origin) {
if (notBlank(uri.getRawUserInfo()) || hasQueryOrFragment(uri)) {
return invalid(
"repository_remote_credentials_unsupported",
"HTTP(S) repository URLs must not include embedded credentials, query strings, or fragments. Use a credential helper or SSH.");
}
if (unsafeUriComponent(uri.getRawAuthority(), uri.getAuthority())
|| unsafeUriComponent(uri.getRawPath(), uri.getPath())) {
return invalid(
"repository_remote_malformed",
"The selected repository URL contains unsupported control characters.");
}
return uriRemote(uri, origin, false);
}
private static RepositorySourceSelection sshRemote(URI uri, RepositorySource.Origin origin) {
if (hasQueryOrFragment(uri)) {
return invalid(
"repository_remote_malformed", "SSH repository URLs must not include query strings or fragments.");
}
String userInfo = uri.getUserInfo();
if (notBlank(userInfo) && userInfo.contains(":")) {
return invalid(
"repository_remote_credentials_unsupported",
"SSH repository URLs must not include password-like user information. Use a username-only SSH URL.");
}
if (unsafeUriComponent(uri.getRawAuthority(), uri.getAuthority())
|| unsafeUriComponent(uri.getRawUserInfo(), uri.getUserInfo())
|| unsafeUriComponent(uri.getRawPath(), uri.getPath())) {
return invalid(
"repository_remote_malformed",
"The selected repository URL contains unsupported control characters.");
}
return uriRemote(uri, origin, true);
}
private static RepositorySourceSelection uriRemote(URI uri, RepositorySource.Origin origin, boolean includeUser) {
String rawPath = uri.getRawPath();
if (blank(uri.getHost()) || blank(rawPath) || "/".equals(rawPath)) {
return invalid(
"repository_remote_malformed",
"The selected repository URL must include a host and repository path.");
}
String repositoryPath = stripSlashes(rawPath);
try {
RepositoryIdentity identity = new RepositoryIdentity(authorityHost(uri), repositoryPath);
return RepositorySourceSelection.selected(new RepositorySource(
RepositorySource.Kind.REMOTE,
origin,
normalizedUri(uri, repositoryPath, includeUser),
identity,
null));
} catch (IllegalArgumentException e) {
return invalid(
"repository_remote_malformed", "The selected repository URL must include a valid repository path.");
}
}
private static RepositorySourceSelection scpRemote(String value, RepositorySource.Origin origin) {
if (value.contains("://") || explicitUriScheme(value)) {
return RepositorySourceSelection.none();
}
int colon = value.indexOf(':');
if (colon <= 0 || colon == value.length() - 1) {
return RepositorySourceSelection.none();
}
String hostPart = value.substring(0, colon);
String rawPath = value.substring(colon + 1);
if (!hostPart.contains("@")) {
return RepositorySourceSelection.none();
}
if (containsWhitespace(hostPart)
|| containsWhitespace(rawPath)
|| unsafeRawComponent(hostPart)
|| unsafeRawComponent(rawPath)
|| rawPath.isBlank()) {
return RepositorySourceSelection.none();
}
int at = hostPart.lastIndexOf('@');
String user = at >= 0 ? hostPart.substring(0, at) : null;
String host = at >= 0 ? hostPart.substring(at + 1) : hostPart;
if ((user != null && !simpleName(user)) || !simpleHost(host)) {
return RepositorySourceSelection.none();
}
String path = stripSlashes(rawPath);
try {
String prefix = user == null ? "" : user + "@";
RepositoryIdentity identity = new RepositoryIdentity(host, path);
return RepositorySourceSelection.selected(new RepositorySource(
RepositorySource.Kind.REMOTE, origin, prefix + identity.host() + ":" + path, identity, null));
} catch (IllegalArgumentException e) {
return invalid(
"repository_remote_malformed",
"The selected SCP-style repository remote must include a valid host and repository path.");
}
}
private static RepositorySourceSelection fileUriPath(String value, RepositorySource.Origin origin) {
try {
URI uri = new URI(value);
if (!"file".equals(lower(uri.getScheme()))
|| hasQueryOrFragment(uri)
|| unsafeUriComponent(uri.getRawPath(), uri.getPath())) {
return invalid(
"repository_path_malformed",
"The selected file repository URL is malformed. Use a valid file URL or local checkout path.");
}
Path path = Path.of(uri).toAbsolutePath().normalize();
return RepositorySourceSelection.selected(
new RepositorySource(RepositorySource.Kind.LOCAL_PATH, origin, path.toString(), null, path));
} catch (IllegalArgumentException | URISyntaxException e) {
return invalid(
"repository_path_malformed",
"The selected file repository URL is malformed. Use a valid file URL or local checkout path.");
}
}
private static RepositorySourceSelection localPath(String value, RepositorySource.Origin origin) {
try {
if (!RepositorySourceText.safePromptLine(stripAngleBrackets(value))) {
return invalid("repository_path_malformed", "The selected local repository path is malformed.");
}
Path path = Path.of(stripAngleBrackets(value)).toAbsolutePath().normalize();
return RepositorySourceSelection.selected(
new RepositorySource(RepositorySource.Kind.LOCAL_PATH, origin, path.toString(), null, path));
} catch (InvalidPathException e) {
return invalid("repository_path_malformed", "The selected local repository path is malformed.");
}
}
private static RepositorySourceSelection invalid(String code, String guidance) {
return RepositorySourceSelection.invalid(new RepositorySourceProblem(code, guidance));
}
private static RepositorySourceSelection malformed(SourceMode mode) {
return switch (mode) {
case LOCAL_PATH -> invalid("repository_path_malformed", "The selected local repository path is malformed.");
case REMOTE_ONLY, REMOTE_OR_FILE, REMOTE_FILE_OR_LOCAL ->
invalid(
"repository_remote_malformed",
"The selected repository URL must include a valid host and repository path.");
};
}
private static List<Declaration> declarations(Card card) {
List<Declaration> declarations = new ArrayList<>();
addDeclarations(declarations, card.title());
addDeclarations(declarations, card.description());
card.comments().stream().map(Card.Comment::text).forEach(text -> addDeclarations(declarations, text));
return List.copyOf(declarations);
}
private static void addDeclarations(List<Declaration> declarations, String text) {
if (text == null || text.isBlank()) {
return;
}
for (String line : text.split("\\R", -1)) {
Matcher labeled = LABELED_SOURCE.matcher(line);
if (labeled.matches()) {
declarations.add(new Declaration(labeled.group(2), labelMode(labeled.group(1))));
}
}
}
private static boolean equivalent(RepositorySourceSelection expected, RepositorySourceSelection actual) {
if (actual.status() != RepositorySourceSelection.Status.SELECTED) {
return false;
}
RepositorySource left = expected.source();
RepositorySource right = actual.source();
return left.kind() == right.kind()
&& left.value().equals(right.value())
&& (left.identity() == null
? right.identity() == null
: left.identity().equals(right.identity()))
&& (left.path() == null ? right.path() == null : left.path().equals(right.path()));
}
private static SourceMode labelMode(String label) {
String normalized =
label.toLowerCase(Locale.ROOT).replaceAll("\\s+", " ").strip();
if (normalized.endsWith(" url")) {
return SourceMode.REMOTE_ONLY;
}
if (normalized.endsWith(" path") || normalized.contains("checkout")) {
return SourceMode.LOCAL_PATH;
}
return SourceMode.REMOTE_FILE_OR_LOCAL;
}
private static String normalizedUri(URI uri, String repositoryPath, boolean includeUser) {
String user = includeUser && notBlank(uri.getRawUserInfo()) ? uri.getRawUserInfo() + "@" : "";
return lower(uri.getScheme()) + "://" + user + authorityHost(uri) + "/" + repositoryPath;
}
private static String authorityHost(URI uri) {
String host = uri.getHost().toLowerCase(Locale.ROOT);
return uri.getPort() < 0 ? host : host + ":" + uri.getPort();
}
private static boolean explicitUriScheme(String value) {
Matcher matcher = URI_SCHEME.matcher(value);
return matcher.find() && matcher.group(1).length() > 1;
}
private static boolean windowsDrivePath(String value) {
return WINDOWS_DRIVE_PATH.matcher(value).matches();
}
private static boolean fileUri(String value) {
return value.regionMatches(true, 0, "file://", 0, "file://".length());
}
private static String stripTokenPunctuation(String value) {
String stripped = value.strip();
while (!stripped.isEmpty() && "<([{".indexOf(stripped.charAt(0)) >= 0) {
stripped = stripped.substring(1);
}
while (!stripped.isEmpty() && ".,;:)>]}".indexOf(stripped.charAt(stripped.length() - 1)) >= 0) {
stripped = stripped.substring(0, stripped.length() - 1);
}
return stripped;
}
private static String stripAngleBrackets(String value) {
String stripped = value.strip();
if (stripped.startsWith("<") && stripped.endsWith(">") && stripped.length() > 2) {
return stripped.substring(1, stripped.length() - 1);
}
return stripped;
}
private static String stripSlashes(String value) {
String stripped = value.strip();
while (stripped.startsWith("/")) {
stripped = stripped.substring(1);
}
while (stripped.endsWith("/")) {
stripped = stripped.substring(0, stripped.length() - 1);
}
return stripped;
}
private static String lower(String value) {
return value == null ? null : value.toLowerCase(Locale.ROOT);
}
private static boolean simpleName(String value) {
return !value.isBlank()
&& value.chars()
.allMatch(character -> Character.isLetterOrDigit(character)
|| character == '.'
|| character == '_'
|| character == '-');
}
private static boolean simpleHost(String value) {
return !value.isBlank()
&& Character.isLetterOrDigit(value.charAt(0))
&& value.chars()
.allMatch(character -> Character.isLetterOrDigit(character)
|| character == '.'
|| character == '_'
|| character == '-');
}
private static boolean containsWhitespace(String value) {
return value.chars().anyMatch(Character::isWhitespace);
}
private static boolean unsafeUriComponent(String rawValue, String decodedValue) {
return unsafeRawComponent(rawValue) || unsafeDecodedComponent(decodedValue);
}
private static boolean unsafeRawComponent(String value) {
return value != null && value.chars().anyMatch(RepositorySourceText::unsafePromptLineCharacter);
}
private static boolean unsafeDecodedComponent(String value) {
return value != null && value.chars().anyMatch(RepositorySourceText::unsafePromptLineCharacter);
}
private static boolean hasQueryOrFragment(URI uri) {
return uri.getRawQuery() != null || uri.getRawFragment() != null;
}
private static boolean notBlank(String value) {
return !blank(value);
}
private static boolean blank(String value) {
return value == null || value.isBlank();
}
private enum SourceMode {
REMOTE_ONLY,
REMOTE_OR_FILE,
REMOTE_FILE_OR_LOCAL,
LOCAL_PATH;
private boolean allowsLocalPath() {
return this == REMOTE_FILE_OR_LOCAL || this == LOCAL_PATH;
}
}
private record Declaration(String value, SourceMode mode) {}
}
Focus area in the original code
The eval should focus especially on this section:
private static List<Declaration> declarations(Card card) {
List<Declaration> declarations = new ArrayList<>();
addDeclarations(declarations, card.title());
addDeclarations(declarations, card.description());
card.comments().stream().map(Card.Comment::text).forEach(text -> addDeclarations(declarations, text));
return List.copyOf(declarations);
}
private static void addDeclarations(List<Declaration> declarations, String text) {
if (text == null || text.isBlank()) {
return;
}
for (String line : text.split("\\R", -1)) {
Matcher labeled = LABELED_SOURCE.matcher(line);
if (labeled.matches()) {
declarations.add(new Declaration(labeled.group(2), labelMode(labeled.group(1))));
}
}
}
Prompt that should be evaluated
Use a prompt in this spirit:
Is this the best way, in terms of performance and readability, of using Java Streams in a Java 25 app? If there is a way to refactor the following code to make it more readable and/or more performant, give me the fully updated code. Feel free to use advanced streams/collectors features.
The prompt should include the full class above or enough of the class that the model has access to these members:
LABELED_SOURCE
Declaration
labelMode(...)
Card.Comment::text
- the existing
declarations(...) and addDeclarations(...) methods
Bad suggestion produced before maintainer correction
The model first suggested replacing split("\\R", -1) with String.lines(), which was a useful direction, but it still used forEach(...) to mutate an externally owned list.
private static List<Declaration> declarations(Card card) {
List<Declaration> declarations = new ArrayList<>();
addDeclarations(declarations, card.title());
addDeclarations(declarations, card.description());
for (Card.Comment comment : card.comments()) {
addDeclarations(declarations, comment.text());
}
return List.copyOf(declarations);
}
private static void addDeclarations(List<Declaration> declarations, String text) {
if (text == null || text.isBlank()) {
return;
}
text.lines().forEach(line -> addDeclaration(declarations, line));
}
private static void addDeclaration(List<Declaration> declarations, String line) {
Matcher labeled = LABELED_SOURCE.matcher(line);
if (labeled.matches()) {
declarations.add(new Declaration(labeled.group(2), labelMode(labeled.group(1))));
}
}
Why the bad suggestion is not good enough
This version is better than the original in one narrow way: String.lines() avoids eagerly splitting the whole text into an array.
However, it is still not the best stream-based refactor because:
- It uses
forEach(...) for side effects.
- The stream is only being used as a line iterator.
- The actual result is still built by mutating a list outside the pipeline.
- It keeps a helper whose job is to mutate the caller's collection.
- It does not express the actual transformation as a single pipeline from input texts to declarations.
The problem is not that forEach(...) is always wrong. The problem is that the user explicitly asked about better stream usage. In that context, a stream pipeline should preferably describe how declarations are produced, not just replace a loop with forEach(...).
Maintainer-preferred code
The preferred implementation uses String.lines() and mapMulti(...).
Required imports:
import java.util.function.Consumer;
import java.util.stream.Stream;
Preferred methods:
private static List<Declaration> declarations(Card card) {
return Stream.concat(
Stream.of(card.title(), card.description()),
card.comments().stream().map(Card.Comment::text))
.filter(text -> text != null && !text.isBlank())
.flatMap(RepositorySourceResolver::declarationsIn)
.toList();
}
private static Stream<Declaration> declarationsIn(String text) {
return text.lines().<Declaration>mapMulti(RepositorySourceResolver::emitDeclaration);
}
private static void emitDeclaration(String line, Consumer<? super Declaration> declarations) {
Matcher labeled = LABELED_SOURCE.matcher(line);
if (labeled.matches()) {
declarations.accept(new Declaration(labeled.group(2), labelMode(labeled.group(1))));
}
}
Why the preferred implementation is better
The preferred implementation represents the operation as a transformation pipeline:
card title, description, and comments
filter blank or null text blocks
split text blocks lazily into lines
emit zero or one Declaration per line
collect into an unmodifiable list
Benefits:
String.lines() avoids the eager array allocation from text.split("\\R", -1).
Stream.toList() produces the result directly from the pipeline.
- No external mutable
List<Declaration> is passed around.
- No
forEach(...) terminal operation is used for collection mutation.
mapMulti(...) matches the exact cardinality of the operation: each input line produces zero or one output declaration.
- The matching logic remains local and readable in
emitDeclaration(...).
Important nuance: the preferred code still uses flatMap(...) at the text-block level:
.flatMap(RepositorySourceResolver::declarationsIn)
That use is acceptable because each text block naturally produces a stream of declarations. The eval should not reject that. The eval should reject flatMap(...) as the line-level zero-or-one declaration mechanism when mapMulti(...) is a better fit.
FlatMap alternative and why it is not the best choice
A possible alternative is:
private static Stream<Declaration> declarationsIn(String text) {
return text.lines()
.flatMap(line -> {
Matcher labeled = LABELED_SOURCE.matcher(line);
if (labeled.matches()) {
return Stream.of(new Declaration(labeled.group(2), labelMode(labeled.group(1))));
}
return Stream.empty();
});
}
This is functionally correct, but it is not the expected best answer for this eval.
Why not:
- It creates a tiny stream for every line: either
Stream.of(...) or Stream.empty().
- It is more verbose than
mapMulti(...) for the zero-or-one output case.
- It hides the simple conditional emission behind stream construction.
- It is exactly the kind of use case where
mapMulti(...) is clearer and avoids per-line nested streams.
The eval should still accept a response that mentions this as a valid alternative, but the expected final implementation should prefer mapMulti(...).
Desired eval behavior
Add an eval that rewards an answer that:
- recognizes that
forEach(...) with external mutation is not the best stream-based refactor here;
- replaces
text.split("\\R", -1) with String.lines() unless full regex \\R behavior is explicitly required;
- avoids passing a mutable
List<Declaration> into helper methods solely so those helpers can mutate it;
- uses a pipeline that returns the declarations via
toList();
- uses
Stream.concat(...) or an equivalent readable approach to combine card.title(), card.description(), and comment text;
- filters out
null and blank text blocks before line processing;
- uses
mapMulti(...) for the line-to-zero-or-one-declaration transformation;
- extracts a small emitter helper such as
emitDeclaration(...) when it improves readability;
- preserves matching behavior with
LABELED_SOURCE.matcher(line).matches();
- preserves declaration construction with
new Declaration(labeled.group(2), labelMode(labeled.group(1)));
- explains why the side-effecting
forEach(...) refactor is weaker than a result-producing pipeline;
- explains why line-level
flatMap(...) is valid but not ideal for zero-or-one output per line;
- distinguishes acceptable
flatMap(...) usage at the text-block level from suboptimal flatMap(...) usage at the line level.
Anti-patterns the eval should reject
Reject answers that:
- simply replace the original loop with
text.lines().forEach(...) and mutate an external list;
- claim that
forEach(...) is the most idiomatic stream-based way to build the result;
- keep the helper method shaped as
addDeclarations(List<Declaration> declarations, String text) when proposing a stream refactor;
- use line-level
flatMap(...) with Stream.of(...) and Stream.empty() as the final preferred answer without discussing mapMulti(...);
- create an intermediate
List<String> of lines;
- keep
text.split("\\R", -1) without explaining the tradeoff;
- accidentally change the label matching rule from
matches() to find();
- drop title, description, or comments from the declaration search;
- fail to filter out
null text blocks;
- return a mutable list where the original returned an unmodifiable copy;
- over-abstract the code into collectors or custom classes that make the transformation harder to understand.
Suggested eval name
mapmulti-line-declaration-extraction
Problem
Add a Java Streams skill eval for a real refactoring case where the model is asked to improve an existing Java 25 class that extracts repository-source declarations from Trello card text.
The important lesson is not "always use streams". The eval should teach that when a stream pipeline is used to transform input into output, it should normally produce the result through the pipeline instead of using
forEach(...)to mutate an external collection.This case also teaches the difference between these three approaches for a line-to-declaration transformation:
forEach(...)with external mutation: works, but is not an idiomatic stream-based refactor.flatMap(...)withStream.of(...)andStream.empty(): valid, but creates a tiny stream for every input line.mapMulti(...): preferred for this case, because each line produces zero or one declaration.The desired eval outcome is that the model replaces the original eager line splitting and mutable accumulation with a
String.lines()plusmapMulti(...)based implementation, not with a side-effectingforEach(...)or a line-levelflatMap(...).Original code provided to the agent
The agent was given the full class below. The relevant methods are
declarations(...)andaddDeclarations(...), but the full class matters because the refactor must preserve surrounding behavior, includingLABELED_SOURCE,Declaration,labelMode(...), and the parsing conflict logic.Full original class
Focus area in the original code
The eval should focus especially on this section:
Prompt that should be evaluated
Use a prompt in this spirit:
The prompt should include the full class above or enough of the class that the model has access to these members:
LABELED_SOURCEDeclarationlabelMode(...)Card.Comment::textdeclarations(...)andaddDeclarations(...)methodsBad suggestion produced before maintainer correction
The model first suggested replacing
split("\\R", -1)withString.lines(), which was a useful direction, but it still usedforEach(...)to mutate an externally owned list.Why the bad suggestion is not good enough
This version is better than the original in one narrow way:
String.lines()avoids eagerly splitting the whole text into an array.However, it is still not the best stream-based refactor because:
forEach(...)for side effects.The problem is not that
forEach(...)is always wrong. The problem is that the user explicitly asked about better stream usage. In that context, a stream pipeline should preferably describe how declarations are produced, not just replace a loop withforEach(...).Maintainer-preferred code
The preferred implementation uses
String.lines()andmapMulti(...).Required imports:
Preferred methods:
Why the preferred implementation is better
The preferred implementation represents the operation as a transformation pipeline:
Benefits:
String.lines()avoids the eager array allocation fromtext.split("\\R", -1).Stream.toList()produces the result directly from the pipeline.List<Declaration>is passed around.forEach(...)terminal operation is used for collection mutation.mapMulti(...)matches the exact cardinality of the operation: each input line produces zero or one output declaration.emitDeclaration(...).Important nuance: the preferred code still uses
flatMap(...)at the text-block level:That use is acceptable because each text block naturally produces a stream of declarations. The eval should not reject that. The eval should reject
flatMap(...)as the line-level zero-or-one declaration mechanism whenmapMulti(...)is a better fit.FlatMap alternative and why it is not the best choice
A possible alternative is:
This is functionally correct, but it is not the expected best answer for this eval.
Why not:
Stream.of(...)orStream.empty().mapMulti(...)for the zero-or-one output case.mapMulti(...)is clearer and avoids per-line nested streams.The eval should still accept a response that mentions this as a valid alternative, but the expected final implementation should prefer
mapMulti(...).Desired eval behavior
Add an eval that rewards an answer that:
forEach(...)with external mutation is not the best stream-based refactor here;text.split("\\R", -1)withString.lines()unless full regex\\Rbehavior is explicitly required;List<Declaration>into helper methods solely so those helpers can mutate it;toList();Stream.concat(...)or an equivalent readable approach to combinecard.title(),card.description(), and comment text;nulland blank text blocks before line processing;mapMulti(...)for the line-to-zero-or-one-declaration transformation;emitDeclaration(...)when it improves readability;LABELED_SOURCE.matcher(line).matches();new Declaration(labeled.group(2), labelMode(labeled.group(1)));forEach(...)refactor is weaker than a result-producing pipeline;flatMap(...)is valid but not ideal for zero-or-one output per line;flatMap(...)usage at the text-block level from suboptimalflatMap(...)usage at the line level.Anti-patterns the eval should reject
Reject answers that:
text.lines().forEach(...)and mutate an external list;forEach(...)is the most idiomatic stream-based way to build the result;addDeclarations(List<Declaration> declarations, String text)when proposing a stream refactor;flatMap(...)withStream.of(...)andStream.empty()as the final preferred answer without discussingmapMulti(...);List<String>of lines;text.split("\\R", -1)without explaining the tradeoff;matches()tofind();nulltext blocks;Suggested eval name
mapmulti-line-declaration-extraction