Hi, Thanks to the feedback
check exists() is not necessary canRead public boolean canRead() Tests whether the application can read the file denoted by this abstract pathname. Returns:true if and only if the file specified by this abstract pathname exists *and* can be read by the application; false otherwise New commit with exists() removed and else removed Antonio Cet e-mail a été envoyé depuis un ordinateur protégé par Avast. www.avast.com <https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B> <#4184246894694592853_DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2> 2016-03-17 10:54 GMT+01:00 sebb <[email protected]>: > On 17 March 2016 at 07:39, vlsi <[email protected]> wrote: > > Github user vlsi commented on a diff in the pull request: > > > > https://github.com/apache/jmeter/pull/167#discussion_r56465359 > > > > --- Diff: src/core/org/apache/jmeter/services/FileServer.java --- > > @@ -418,16 +418,20 @@ private BufferedReader getReader(String alias, > boolean recycle, boolean firstLin > > } > > > > private BufferedReader createBufferedReader(FileEntry > fileEntry) throws IOException { > > - FileInputStream fis = new FileInputStream(fileEntry.file); > > - InputStreamReader isr = null; > > - // If file encoding is specified, read using that encoding, > otherwise use default platform encoding > > - String charsetName = fileEntry.charSetEncoding; > > - if(!JOrphanUtils.isBlank(charsetName)) { > > - isr = new InputStreamReader(fis, charsetName); > > + if (!fileEntry.file.exists() || !fileEntry.file.canRead() > || !fileEntry.file.isFile()) { > > Surely there's no need to check exists() if you check canRead()? > > > + throw new IllegalArgumentException("File "+ > fileEntry.file.getName()+ " must exist and be readable"); > > } else { > > --- End diff -- > > > > Please remove `else` and the following braces. > > It would avoid indenting the following code. > > > > > > --- > > If your project is set up for it, you can reply to this email and have > your > > reply appear on GitHub as well. If your project does not have this > feature > > enabled and wishes so, or if the feature is enabled but not working, > please > > contact infrastructure at [email protected] or file a JIRA > ticket > > with INFRA. > > --- >
