[
https://issues.apache.org/jira/browse/FLINK-32128?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Seungchul Lee updated FLINK-32128:
----------------------------------
Description:
While using the Flink in our company, I would like to share a small suggestion
in the test code. I understand that it may be a minor issue, but I would like
to raise it and share my thoughts as one of the Flink's user.
As you already know, ArrayList is a mutable container class.
To prevent callers from being able to mutable its internal data, the current
code copies the entires data into new ArrayList as below.
{code:java}
public class TestingReaderContext implements SourceReaderContext {
private final SourceReaderMetricGroup metrics;
private final Configuration config;
private final ArrayList<SourceEvent> sentEvents = new ArrayList<>();
....
public List<SourceEvent> getSentEvents() {
return new ArrayList<>(sentEvents);
}
....
} {code}
I would like to suggest that alternatively, if possible, it is better to return
some other implementation that the class can use to enforce its invariants.
For example, rather than copying its entire dataset in the current test code, I
would like to use an unmodifiable implementation as shown in below.
{code:java}
public class TestingReaderContext implements SourceReaderContext {
private final SourceReaderMetricGroup metrics;
private final Configuration config;
private final ArrayList<SourceEvent> sentEvents = new ArrayList<>();
....
public List<SourceEvent> getSentEvents() {
return Collections.unmodifiableList(sentEvents);
}
....
} {code}
Since the callers that uses the sentEvents list are only used for reading its
internal data in the test code, it would be better to use an unmodifiable
implementation.
The entire code can be found in the TestingReaderContext class.
While it may be a small suggestion, I raised the issue with the intention of
making the current test code more robust.
Thanks.
was:
While using the Flink in our company, I would like to share a small suggestion
in the test code. I understand that it may be a minor issue, but I would like
to raise it and share my thoughts as one of the Flink's user.
As you already know, ArrayList is a mutable container class.
To prevent callers from being able to mutable its internal data, the current
code copies the entires data into new ArrayList as below.
{code:java}
public class TestingReaderContext implements SourceReaderContext {
private final SourceReaderMetricGroup metrics;
private final Configuration config;
private final ArrayList<SourceEvent> sentEvents = new ArrayList<>();
....
public List<SourceEvent> getSentEvents() {
return new ArrayList<>(sentEvents);
}
....
} {code}
I would like to suggest that alternatively, if possible, it is better to return
some other implementation that the class can use to enforce its invariants.
For example, rather than copying its entire dataset in the current test code, I
would like to use an unmodifiable implementation as shown in below.
{code:java}
public class TestingReaderContext implements SourceReaderContext {
private final SourceReaderMetricGroup metrics;
private final Configuration config;
private final ArrayList<SourceEvent> sentEvents = new ArrayList<>();
....
public List<SourceEvent> getSentEvents() {
return Collections.unmodifiableList(sentEvents);
}
....
} {code}
Since the callers that uses the sentEvents list are only used for reading its
internal data in the test code, it would be better to use an unmodifiable
implementation.
The entire code can be found in the TestingReaderContext class.
While it may be a small suggestion, I raised the issue with the intention of
making the current test code more robust.
Thanks.
> Unmodifiable implementation for getter method in test function
> --------------------------------------------------------------
>
> Key: FLINK-32128
> URL: https://issues.apache.org/jira/browse/FLINK-32128
> Project: Flink
> Issue Type: Improvement
> Components: Tests
> Affects Versions: 1.17.0
> Reporter: Seungchul Lee
> Priority: Minor
> Labels: test-stability
> Fix For: 1.18.0
>
> Original Estimate: 2h
> Remaining Estimate: 2h
>
> While using the Flink in our company, I would like to share a small
> suggestion in the test code. I understand that it may be a minor issue, but I
> would like to raise it and share my thoughts as one of the Flink's user.
>
> As you already know, ArrayList is a mutable container class.
> To prevent callers from being able to mutable its internal data, the current
> code copies the entires data into new ArrayList as below.
>
>
> {code:java}
> public class TestingReaderContext implements SourceReaderContext {
> private final SourceReaderMetricGroup metrics;
> private final Configuration config;
> private final ArrayList<SourceEvent> sentEvents = new ArrayList<>();
> ....
> public List<SourceEvent> getSentEvents() {
> return new ArrayList<>(sentEvents);
> }
> ....
> } {code}
>
> I would like to suggest that alternatively, if possible, it is better to
> return some other implementation that the class can use to enforce its
> invariants.
> For example, rather than copying its entire dataset in the current test code,
> I would like to use an unmodifiable implementation as shown in below.
>
>
> {code:java}
> public class TestingReaderContext implements SourceReaderContext {
> private final SourceReaderMetricGroup metrics;
> private final Configuration config;
> private final ArrayList<SourceEvent> sentEvents = new ArrayList<>();
> ....
> public List<SourceEvent> getSentEvents() {
> return Collections.unmodifiableList(sentEvents);
> }
> ....
> } {code}
>
>
> Since the callers that uses the sentEvents list are only used for reading its
> internal data in the test code, it would be better to use an unmodifiable
> implementation.
>
> The entire code can be found in the TestingReaderContext class.
>
> While it may be a small suggestion, I raised the issue with the intention of
> making the current test code more robust.
>
> Thanks.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
