Trying to minimize duplicate code in a loop












-1















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.










share|improve this question

























  • 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











  • 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











  • 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
















-1















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.










share|improve this question

























  • 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











  • 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











  • 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














-1












-1








-1








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.










share|improve this question
















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






share|improve this question















share|improve this question













share|improve this question




share|improve this question








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 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











  • 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











  • 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













  • 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











  • @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

















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












2 Answers
2






active

oldest

votes


















0














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();
}
});
}





share|improve this answer
























  • This looks like a very good solution, Thank you.

    – SamzSakerz
    Nov 26 '18 at 13:33



















0














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;
}
}





share|improve this answer























    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
    });


    }
    });














    draft saved

    draft discarded


















    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









    0














    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();
    }
    });
    }





    share|improve this answer
























    • This looks like a very good solution, Thank you.

      – SamzSakerz
      Nov 26 '18 at 13:33
















    0














    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();
    }
    });
    }





    share|improve this answer
























    • This looks like a very good solution, Thank you.

      – SamzSakerz
      Nov 26 '18 at 13:33














    0












    0








    0







    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();
    }
    });
    }





    share|improve this answer













    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();
    }
    });
    }






    share|improve this answer












    share|improve this answer



    share|improve this answer










    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



















    • 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













    0














    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;
    }
    }





    share|improve this answer




























      0














      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;
      }
      }





      share|improve this answer


























        0












        0








        0







        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;
        }
        }





        share|improve this answer













        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;
        }
        }






        share|improve this answer












        share|improve this answer



        share|improve this answer










        answered Nov 26 '18 at 5:58









        Perdi EstaquelPerdi Estaquel

        6701519




        6701519






























            draft saved

            draft discarded




















































            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.




            draft saved


            draft discarded














            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





















































            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







            Popular posts from this blog

            404 Error Contact Form 7 ajax form submitting

            How to know if a Active Directory user can login interactively

            TypeError: fit_transform() missing 1 required positional argument: 'X'