Trying to minimize duplicate code in a loop
I'm converting my java for loop that I wrote into a java stream, The for loop had duplicate code that I think can be fixed with a stream, This is what I had going on, on my old loop.
try (DirectoryStream<Path> stream = Files.newDirectoryStream(path)) {
for (Path entry : stream) {
String fileName = entry.getFileName().toString();
if (Files.isDirectory(entry)) {
grepFilesAndFolders(entry);
} else if (shouldChange(entry)) {
//Doing all the foreach work here
}
}
}
}
and this is what I have currently with my stream
Files.walk(path)
.filter(this::shouldChange)
.forEach(p -> {
try {
String fileName = p.getFileName().toString();
Charset charset = StandardCharsets.UTF_8;
String content = new String(Files.readAllBytes(p), charset);
boolean contentChanged = false;
int count = countMatches(content, getName(oldName, '1'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '1'),
fileName,
getName(newName, '1'),
content);
contentChanged = true;
}
count = countMatches(content, getName(oldName, '2'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '2'),
fileName,
getName(newName, '2'), content);
contentChanged = true;
}
if (contentChanged) {
Files.write(p, content.getBytes(charset));
}
} catch (IOException e) {
e.printStackTrace();
}
});
What I am trying to do is try to get rid of the duplicate code which is the following.
int count = countMatches(content, getName(oldName, '1'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '1'),
fileName,
getName(newName, '1'),
content);
contentChanged = true;
}
count = countMatches(content, getName(oldName, '2'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '2'),
fileName,
getName(newName, '2'), content);
contentChanged = true;
}
if (contentChanged) {
Files.write(p, content.getBytes(charset));
}
What I would assume is what I can do is make the replace in new method and then map
it to the stream but that way I cant tract of the contentChanged
variable. But I don't know how to go about this, So far I only have the stream and filter laid out.
java java-stream
|
show 1 more comment
I'm converting my java for loop that I wrote into a java stream, The for loop had duplicate code that I think can be fixed with a stream, This is what I had going on, on my old loop.
try (DirectoryStream<Path> stream = Files.newDirectoryStream(path)) {
for (Path entry : stream) {
String fileName = entry.getFileName().toString();
if (Files.isDirectory(entry)) {
grepFilesAndFolders(entry);
} else if (shouldChange(entry)) {
//Doing all the foreach work here
}
}
}
}
and this is what I have currently with my stream
Files.walk(path)
.filter(this::shouldChange)
.forEach(p -> {
try {
String fileName = p.getFileName().toString();
Charset charset = StandardCharsets.UTF_8;
String content = new String(Files.readAllBytes(p), charset);
boolean contentChanged = false;
int count = countMatches(content, getName(oldName, '1'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '1'),
fileName,
getName(newName, '1'),
content);
contentChanged = true;
}
count = countMatches(content, getName(oldName, '2'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '2'),
fileName,
getName(newName, '2'), content);
contentChanged = true;
}
if (contentChanged) {
Files.write(p, content.getBytes(charset));
}
} catch (IOException e) {
e.printStackTrace();
}
});
What I am trying to do is try to get rid of the duplicate code which is the following.
int count = countMatches(content, getName(oldName, '1'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '1'),
fileName,
getName(newName, '1'),
content);
contentChanged = true;
}
count = countMatches(content, getName(oldName, '2'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '2'),
fileName,
getName(newName, '2'), content);
contentChanged = true;
}
if (contentChanged) {
Files.write(p, content.getBytes(charset));
}
What I would assume is what I can do is make the replace in new method and then map
it to the stream but that way I cant tract of the contentChanged
variable. But I don't know how to go about this, So far I only have the stream and filter laid out.
java java-stream
Can you refactor this code block into a method likeprocessMatches(content, oldNmae, newNmae, fileName,1)
that returns sayPair<Boolean, String>
?
– c0der
Nov 26 '18 at 5:26
I don't see any relationship between the loop code and the stream code.
– shmosel
Nov 26 '18 at 6:16
Can you explain what the expressions likegetName(oldName, '1')
orgetName(newName, '2')
are supposed to do? There is nooldName
nornewName
declared or assigned in this code, so if these inputs are never changing during the entire operation, there is no no reason to repeat these operations multiple times for each file.
– Holger
Nov 26 '18 at 12:50
@Holger they are global variables, I am trying to replace everystring in a file that matches oldName seeded with1
or2
with newName
– SamzSakerz
Nov 26 '18 at 13:32
That still sounds like “oldName seeded with 1 or 2” does not need to get recomputed multiple times per file.
– Holger
Nov 26 '18 at 13:42
|
show 1 more comment
I'm converting my java for loop that I wrote into a java stream, The for loop had duplicate code that I think can be fixed with a stream, This is what I had going on, on my old loop.
try (DirectoryStream<Path> stream = Files.newDirectoryStream(path)) {
for (Path entry : stream) {
String fileName = entry.getFileName().toString();
if (Files.isDirectory(entry)) {
grepFilesAndFolders(entry);
} else if (shouldChange(entry)) {
//Doing all the foreach work here
}
}
}
}
and this is what I have currently with my stream
Files.walk(path)
.filter(this::shouldChange)
.forEach(p -> {
try {
String fileName = p.getFileName().toString();
Charset charset = StandardCharsets.UTF_8;
String content = new String(Files.readAllBytes(p), charset);
boolean contentChanged = false;
int count = countMatches(content, getName(oldName, '1'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '1'),
fileName,
getName(newName, '1'),
content);
contentChanged = true;
}
count = countMatches(content, getName(oldName, '2'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '2'),
fileName,
getName(newName, '2'), content);
contentChanged = true;
}
if (contentChanged) {
Files.write(p, content.getBytes(charset));
}
} catch (IOException e) {
e.printStackTrace();
}
});
What I am trying to do is try to get rid of the duplicate code which is the following.
int count = countMatches(content, getName(oldName, '1'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '1'),
fileName,
getName(newName, '1'),
content);
contentChanged = true;
}
count = countMatches(content, getName(oldName, '2'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '2'),
fileName,
getName(newName, '2'), content);
contentChanged = true;
}
if (contentChanged) {
Files.write(p, content.getBytes(charset));
}
What I would assume is what I can do is make the replace in new method and then map
it to the stream but that way I cant tract of the contentChanged
variable. But I don't know how to go about this, So far I only have the stream and filter laid out.
java java-stream
I'm converting my java for loop that I wrote into a java stream, The for loop had duplicate code that I think can be fixed with a stream, This is what I had going on, on my old loop.
try (DirectoryStream<Path> stream = Files.newDirectoryStream(path)) {
for (Path entry : stream) {
String fileName = entry.getFileName().toString();
if (Files.isDirectory(entry)) {
grepFilesAndFolders(entry);
} else if (shouldChange(entry)) {
//Doing all the foreach work here
}
}
}
}
and this is what I have currently with my stream
Files.walk(path)
.filter(this::shouldChange)
.forEach(p -> {
try {
String fileName = p.getFileName().toString();
Charset charset = StandardCharsets.UTF_8;
String content = new String(Files.readAllBytes(p), charset);
boolean contentChanged = false;
int count = countMatches(content, getName(oldName, '1'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '1'),
fileName,
getName(newName, '1'),
content);
contentChanged = true;
}
count = countMatches(content, getName(oldName, '2'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '2'),
fileName,
getName(newName, '2'), content);
contentChanged = true;
}
if (contentChanged) {
Files.write(p, content.getBytes(charset));
}
} catch (IOException e) {
e.printStackTrace();
}
});
What I am trying to do is try to get rid of the duplicate code which is the following.
int count = countMatches(content, getName(oldName, '1'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '1'),
fileName,
getName(newName, '1'),
content);
contentChanged = true;
}
count = countMatches(content, getName(oldName, '2'));
if (count > 0) {
content = replaceName(count,
getName(oldName, '2'),
fileName,
getName(newName, '2'), content);
contentChanged = true;
}
if (contentChanged) {
Files.write(p, content.getBytes(charset));
}
What I would assume is what I can do is make the replace in new method and then map
it to the stream but that way I cant tract of the contentChanged
variable. But I don't know how to go about this, So far I only have the stream and filter laid out.
java java-stream
java java-stream
edited Nov 26 '18 at 6:16
shmosel
36.7k43996
36.7k43996
asked Nov 26 '18 at 3:13
SamzSakerzSamzSakerz
1,6691627
1,6691627
Can you refactor this code block into a method likeprocessMatches(content, oldNmae, newNmae, fileName,1)
that returns sayPair<Boolean, String>
?
– c0der
Nov 26 '18 at 5:26
I don't see any relationship between the loop code and the stream code.
– shmosel
Nov 26 '18 at 6:16
Can you explain what the expressions likegetName(oldName, '1')
orgetName(newName, '2')
are supposed to do? There is nooldName
nornewName
declared or assigned in this code, so if these inputs are never changing during the entire operation, there is no no reason to repeat these operations multiple times for each file.
– Holger
Nov 26 '18 at 12:50
@Holger they are global variables, I am trying to replace everystring in a file that matches oldName seeded with1
or2
with newName
– SamzSakerz
Nov 26 '18 at 13:32
That still sounds like “oldName seeded with 1 or 2” does not need to get recomputed multiple times per file.
– Holger
Nov 26 '18 at 13:42
|
show 1 more comment
Can you refactor this code block into a method likeprocessMatches(content, oldNmae, newNmae, fileName,1)
that returns sayPair<Boolean, String>
?
– c0der
Nov 26 '18 at 5:26
I don't see any relationship between the loop code and the stream code.
– shmosel
Nov 26 '18 at 6:16
Can you explain what the expressions likegetName(oldName, '1')
orgetName(newName, '2')
are supposed to do? There is nooldName
nornewName
declared or assigned in this code, so if these inputs are never changing during the entire operation, there is no no reason to repeat these operations multiple times for each file.
– Holger
Nov 26 '18 at 12:50
@Holger they are global variables, I am trying to replace everystring in a file that matches oldName seeded with1
or2
with newName
– SamzSakerz
Nov 26 '18 at 13:32
That still sounds like “oldName seeded with 1 or 2” does not need to get recomputed multiple times per file.
– Holger
Nov 26 '18 at 13:42
Can you refactor this code block into a method like
processMatches(content, oldNmae, newNmae, fileName,1)
that returns say Pair<Boolean, String>
?– c0der
Nov 26 '18 at 5:26
Can you refactor this code block into a method like
processMatches(content, oldNmae, newNmae, fileName,1)
that returns say Pair<Boolean, String>
?– c0der
Nov 26 '18 at 5:26
I don't see any relationship between the loop code and the stream code.
– shmosel
Nov 26 '18 at 6:16
I don't see any relationship between the loop code and the stream code.
– shmosel
Nov 26 '18 at 6:16
Can you explain what the expressions like
getName(oldName, '1')
or getName(newName, '2')
are supposed to do? There is no oldName
nor newName
declared or assigned in this code, so if these inputs are never changing during the entire operation, there is no no reason to repeat these operations multiple times for each file.– Holger
Nov 26 '18 at 12:50
Can you explain what the expressions like
getName(oldName, '1')
or getName(newName, '2')
are supposed to do? There is no oldName
nor newName
declared or assigned in this code, so if these inputs are never changing during the entire operation, there is no no reason to repeat these operations multiple times for each file.– Holger
Nov 26 '18 at 12:50
@Holger they are global variables, I am trying to replace everystring in a file that matches oldName seeded with
1
or 2
with newName– SamzSakerz
Nov 26 '18 at 13:32
@Holger they are global variables, I am trying to replace everystring in a file that matches oldName seeded with
1
or 2
with newName– SamzSakerz
Nov 26 '18 at 13:32
That still sounds like “oldName seeded with 1 or 2” does not need to get recomputed multiple times per file.
– Holger
Nov 26 '18 at 13:42
That still sounds like “oldName seeded with 1 or 2” does not need to get recomputed multiple times per file.
– Holger
Nov 26 '18 at 13:42
|
show 1 more comment
2 Answers
2
active
oldest
votes
private String replaceName(String content, String fileName, char ch) {
if (countMatches(content, getName(oldName, ch)) > 0)
content = replaceName(count, getName(oldName, ch), fileName, getName(newName, ch), content);
return content;
}
Files.walk(path)
.filter(this::shouldChange)
.forEach(p -> {
try {
String fileName = p.getFileName().toString();
final String content = new String(Files.readAllBytes(p), StandardCharsets.UTF_8);
String changedContent = replaceName(content, fileName, '1');
changedContent = replaceName(content, fileName, '2');
if (!content.equals(changedContent))
Files.write(p, content.getBytes(StandardCharsets.UTF_8));
} catch(IOException e) {
e.printStackTrace();
}
});
}
This looks like a very good solution, Thank you.
– SamzSakerz
Nov 26 '18 at 13:33
add a comment |
for (char i = '1'; i < '3'; ++i) {
int count = countMatches(content, getName(oldName, i));
if (count > 0) {
content = replaceName(count,
getName(oldName, i),
fileName,
getName(newName, i),
content);
contentChanged = true;
}
}
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "1"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: true,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: 10,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53474338%2ftrying-to-minimize-duplicate-code-in-a-loop%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
private String replaceName(String content, String fileName, char ch) {
if (countMatches(content, getName(oldName, ch)) > 0)
content = replaceName(count, getName(oldName, ch), fileName, getName(newName, ch), content);
return content;
}
Files.walk(path)
.filter(this::shouldChange)
.forEach(p -> {
try {
String fileName = p.getFileName().toString();
final String content = new String(Files.readAllBytes(p), StandardCharsets.UTF_8);
String changedContent = replaceName(content, fileName, '1');
changedContent = replaceName(content, fileName, '2');
if (!content.equals(changedContent))
Files.write(p, content.getBytes(StandardCharsets.UTF_8));
} catch(IOException e) {
e.printStackTrace();
}
});
}
This looks like a very good solution, Thank you.
– SamzSakerz
Nov 26 '18 at 13:33
add a comment |
private String replaceName(String content, String fileName, char ch) {
if (countMatches(content, getName(oldName, ch)) > 0)
content = replaceName(count, getName(oldName, ch), fileName, getName(newName, ch), content);
return content;
}
Files.walk(path)
.filter(this::shouldChange)
.forEach(p -> {
try {
String fileName = p.getFileName().toString();
final String content = new String(Files.readAllBytes(p), StandardCharsets.UTF_8);
String changedContent = replaceName(content, fileName, '1');
changedContent = replaceName(content, fileName, '2');
if (!content.equals(changedContent))
Files.write(p, content.getBytes(StandardCharsets.UTF_8));
} catch(IOException e) {
e.printStackTrace();
}
});
}
This looks like a very good solution, Thank you.
– SamzSakerz
Nov 26 '18 at 13:33
add a comment |
private String replaceName(String content, String fileName, char ch) {
if (countMatches(content, getName(oldName, ch)) > 0)
content = replaceName(count, getName(oldName, ch), fileName, getName(newName, ch), content);
return content;
}
Files.walk(path)
.filter(this::shouldChange)
.forEach(p -> {
try {
String fileName = p.getFileName().toString();
final String content = new String(Files.readAllBytes(p), StandardCharsets.UTF_8);
String changedContent = replaceName(content, fileName, '1');
changedContent = replaceName(content, fileName, '2');
if (!content.equals(changedContent))
Files.write(p, content.getBytes(StandardCharsets.UTF_8));
} catch(IOException e) {
e.printStackTrace();
}
});
}
private String replaceName(String content, String fileName, char ch) {
if (countMatches(content, getName(oldName, ch)) > 0)
content = replaceName(count, getName(oldName, ch), fileName, getName(newName, ch), content);
return content;
}
Files.walk(path)
.filter(this::shouldChange)
.forEach(p -> {
try {
String fileName = p.getFileName().toString();
final String content = new String(Files.readAllBytes(p), StandardCharsets.UTF_8);
String changedContent = replaceName(content, fileName, '1');
changedContent = replaceName(content, fileName, '2');
if (!content.equals(changedContent))
Files.write(p, content.getBytes(StandardCharsets.UTF_8));
} catch(IOException e) {
e.printStackTrace();
}
});
}
answered Nov 26 '18 at 7:29
oleg.cherednikoleg.cherednik
7,10021119
7,10021119
This looks like a very good solution, Thank you.
– SamzSakerz
Nov 26 '18 at 13:33
add a comment |
This looks like a very good solution, Thank you.
– SamzSakerz
Nov 26 '18 at 13:33
This looks like a very good solution, Thank you.
– SamzSakerz
Nov 26 '18 at 13:33
This looks like a very good solution, Thank you.
– SamzSakerz
Nov 26 '18 at 13:33
add a comment |
for (char i = '1'; i < '3'; ++i) {
int count = countMatches(content, getName(oldName, i));
if (count > 0) {
content = replaceName(count,
getName(oldName, i),
fileName,
getName(newName, i),
content);
contentChanged = true;
}
}
add a comment |
for (char i = '1'; i < '3'; ++i) {
int count = countMatches(content, getName(oldName, i));
if (count > 0) {
content = replaceName(count,
getName(oldName, i),
fileName,
getName(newName, i),
content);
contentChanged = true;
}
}
add a comment |
for (char i = '1'; i < '3'; ++i) {
int count = countMatches(content, getName(oldName, i));
if (count > 0) {
content = replaceName(count,
getName(oldName, i),
fileName,
getName(newName, i),
content);
contentChanged = true;
}
}
for (char i = '1'; i < '3'; ++i) {
int count = countMatches(content, getName(oldName, i));
if (count > 0) {
content = replaceName(count,
getName(oldName, i),
fileName,
getName(newName, i),
content);
contentChanged = true;
}
}
answered Nov 26 '18 at 5:58
Perdi EstaquelPerdi Estaquel
6701519
6701519
add a comment |
add a comment |
Thanks for contributing an answer to Stack Overflow!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53474338%2ftrying-to-minimize-duplicate-code-in-a-loop%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Can you refactor this code block into a method like
processMatches(content, oldNmae, newNmae, fileName,1)
that returns sayPair<Boolean, String>
?– c0der
Nov 26 '18 at 5:26
I don't see any relationship between the loop code and the stream code.
– shmosel
Nov 26 '18 at 6:16
Can you explain what the expressions like
getName(oldName, '1')
orgetName(newName, '2')
are supposed to do? There is nooldName
nornewName
declared or assigned in this code, so if these inputs are never changing during the entire operation, there is no no reason to repeat these operations multiple times for each file.– Holger
Nov 26 '18 at 12:50
@Holger they are global variables, I am trying to replace everystring in a file that matches oldName seeded with
1
or2
with newName– SamzSakerz
Nov 26 '18 at 13:32
That still sounds like “oldName seeded with 1 or 2” does not need to get recomputed multiple times per file.
– Holger
Nov 26 '18 at 13:42