Skip to content

Commit

Permalink
Fix concurrency bug v2,fix was missing from other constructor
Browse files Browse the repository at this point in the history
  • Loading branch information
@domoritz
domoritz committed Mar 17, 2014
1 parent b24235f commit 5700f94
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/edu/washington/escience/myria/CsvTupleWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ public class CsvTupleWriter implements TupleWriter {
* @param out the {@link OutputStream} to which the data will be written.
*/
public CsvTupleWriter(final OutputStream out) {
csvWriter = new CsvListWriter(new BufferedWriter(new OutputStreamWriter(out)), CsvPreference.STANDARD_PREFERENCE);
final CsvPreference pref =
new CsvPreference.Builder(CsvPreference.STANDARD_PREFERENCE).useEncoder(new DefaultCsvEncoder()).build();
csvWriter = new CsvListWriter(new BufferedWriter(new OutputStreamWriter(out)), pref);
}

/**
Expand Down

7 comments on commit 5700f94

@dhalperi
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to have one constructor call the other to simplify the code, remove redundancy, and prevent similar issues in the future?

@domoritz
Copy link
Member Author

Choose a reason for hiding this comment

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

I would prefer not to in this case because the preference should be built without setting what does not have to be set. IMHO.

@dhalperi
Copy link
Member

Choose a reason for hiding this comment

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

so then a private constructor that takes OutputStream and CsvPreference and does the rest?

@domoritz
Copy link
Member Author

@domoritz domoritz commented on 5700f94 Mar 20, 2014 via email

Choose a reason for hiding this comment

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

@dhalperi
Copy link
Member

Choose a reason for hiding this comment

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

Hmm? If you used this part

final CsvPreference pref =
 +        new CsvPreference.Builder(CsvPreference.STANDARD_PREFERENCE).useEncoder(new DefaultCsvEncoder()).build();
 +    csvWriter = new CsvListWriter(new BufferedWriter(new OutputStreamWriter(out)), pref);

but then replaced the CsvPreference.STANDARD_PREFERENCE with the input it should work.

@dhalperi
Copy link
Member

Choose a reason for hiding this comment

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

I mean, fine, it just seems like the redundancy bit us once and this is a fair bit of complex and redundant code:

.useEncoder(new DefaultCsvEncoder()).build();
    csvWriter = new CsvListWriter(new BufferedWriter(new OutputStreamWriter(out)), separatorPreference);

@domoritz
Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 232df9a.

Please sign in to comment.