Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

[eclipse/xtext#1777] ported xtend code 2 java #629

Merged
merged 1 commit into from
Jul 27, 2020
Merged

Conversation

cdietrich
Copy link
Member

[eclipse/xtext#1777] ported xtend code 2 java
Signed-off-by: Christian Dietrich christian.dietrich@itemis.de

@cdietrich cdietrich added this to the Release_2.23 milestone Jul 17, 2020
@cdietrich cdietrich requested a review from szarnekow July 17, 2020 09:02
cache.put(fileName, null);
return null;
}
ClassFileReader reader = ClassFileReader.read(url.openStream(), fileName);
Copy link

Choose a reason for hiding this comment

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

use try-with-resources for url.openStream()?

Copy link

Choose a reason for hiding this comment

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

				try (InputStream is = url.openStream()) {
					ClassFileReader reader = ClassFileReader.read(is, fileName);
					NameEnvironmentAnswer result = new NameEnvironmentAnswer(reader, null);
					cache.put(fileName, result);
					return result;
				}

@cdietrich cdietrich force-pushed the cd_testingX2j branch 2 times, most recently from 8b10fd1 to e8f0a42 Compare July 17, 2020 10:18
ListExtensions.map(Arrays.asList(sources),
(JavaSource it) -> new CompilationUnit(it.getCode().toCharArray(), it.getFileName(), null)),
ICompilationUnit.class));
compiler.compile(units);
Copy link

Choose a reason for hiding this comment

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

refactor to:

ICompilationUnit[] units = Stream.of(sources).map(it -> new CompilationUnit(it.getCode().toCharArray(), it.getFileName(), null)).toArray(ICompilationUnit[]::new);

Copy link
Member Author

Choose a reason for hiding this comment

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

this is not about refactoring everything

Copy link

Choose a reason for hiding this comment

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

The current code is way more hard to read. Above statement does the same.

Comment on lines +100 to +104
message.append("PROBLEMS : ");
message.newLine();
message.append("\t");
message.append(Joiner.on("\n").join(result.getCompilationProblems()), "\t");
message.newLineIfNotEmpty();
Copy link

Choose a reason for hiding this comment

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

extract to method, sequence is used twice

Copy link
Member Author

Choose a reason for hiding this comment

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

:(

Comment on lines +73 to +77
message.append("Java code compiled with errors:");
message.newLine();
message.append(Joiner.on("\n").join(
Iterables.filter(result.getCompilationProblems(), CategorizedProblem::isError)));
message.newLineIfNotEmpty();
Copy link

Choose a reason for hiding this comment

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

extract to method, sequence is used twice

Comment on lines +176 to +178
sourceCode.append("public class __Generated implements org.eclipse.xtext.xbase.lib.Functions.Function0<");
sourceCode.append(returnType.getName());
sourceCode.append("> {");
Copy link

Choose a reason for hiding this comment

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

This may read better with

sourceCode.append(String.format("public class __Generated implements org.eclipse.xtext.xbase.lib.Functions.Function0<%s> {", returnType.getName())

If you agree there are other line candidates.

@cdietrich cdietrich force-pushed the cd_testingX2j branch 3 times, most recently from 8b15a76 to a515e57 Compare July 17, 2020 10:44
@cdietrich
Copy link
Member Author

ping @kthoms @szarnekow

Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
@cdietrich cdietrich merged commit 53c6cc4 into master Jul 27, 2020
@cdietrich cdietrich deleted the cd_testingX2j branch July 27, 2020 12:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants