jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Antonio Gomes Rodrigues <ra0...@gmail.com>
Subject Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions
Date Thu, 17 Mar 2016 10:46:14 GMT
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 <sebbaz@gmail.com>:

> On 17 March 2016 at 07:39, vlsi <git@git.apache.org> 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 infrastructure@apache.org or file a JIRA
> ticket
> > with INFRA.
> > ---
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message