Feedback welcome: reworking the examples with ParameterTools

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Feedback welcome: reworking the examples with ParameterTools

stefanobaghino
Hello dev,

I'm currently working on [FLINK-2021] Rework examples to use ParameterTool
<https://issues.apache.org/jira/browse/FLINK-2021> and we're trying to
improve the readability of the K-Means example until we reach a
satisfactory starting point to rework the other examples as well. I
think Robert's
improvements
<https://github.com/rmetzger/flink/blob/4c3b569b522172043fe1f49da2858bae37519eef/flink-examples/flink-examples-batch/src/main/java/org/apache/flink/examples/java/clustering/KMeans.java>
(not relying on RequiredParameters and removal of static variables, in
particular) are good enough to be that starting point and integrated those
changes in the PR <https://github.com/apache/flink/pull/1536>.

Before moving on I'd like to have a feedback on the work done so far.
Also: what would you think of moving POJOs and user functions to separate
files to declutter the main class? I've noticed that both in the examples
and in the training solutions there's a general tendency to keep everything
in one place, however my feeling is that splitting the classes would keep
files short and readable, while allowing quick navigation between
files/classes in most editors without having to scroll through the file to
reach the code you're interested in.

--
BR,
Stefano Baghino

Software Engineer @ Radicalbit
Reply | Threaded
Open this post in threaded view
|

Re: Feedback welcome: reworking the examples with ParameterTools

Aljoscha Krettek-2
Hi,
the changes to the KMeans example look good so far. About moving everything to external classes, IMHO we should do it, but I can also see why it is nice to have the whole example contained in one file. So let’s see what the others think.

Cheers,
Aljoscha

> On 21 Jan 2016, at 18:04, Stefano Baghino <[hidden email]> wrote:
>
> Hello dev,
>
> I'm currently working on [FLINK-2021] Rework examples to use ParameterTool
> <https://issues.apache.org/jira/browse/FLINK-2021> and we're trying to
> improve the readability of the K-Means example until we reach a
> satisfactory starting point to rework the other examples as well. I
> think Robert's
> improvements
> <https://github.com/rmetzger/flink/blob/4c3b569b522172043fe1f49da2858bae37519eef/flink-examples/flink-examples-batch/src/main/java/org/apache/flink/examples/java/clustering/KMeans.java>
> (not relying on RequiredParameters and removal of static variables, in
> particular) are good enough to be that starting point and integrated those
> changes in the PR <https://github.com/apache/flink/pull/1536>.
>
> Before moving on I'd like to have a feedback on the work done so far.
> Also: what would you think of moving POJOs and user functions to separate
> files to declutter the main class? I've noticed that both in the examples
> and in the training solutions there's a general tendency to keep everything
> in one place, however my feeling is that splitting the classes would keep
> files short and readable, while allowing quick navigation between
> files/classes in most editors without having to scroll through the file to
> reach the code you're interested in.
>
> --
> BR,
> Stefano Baghino
>
> Software Engineer @ Radicalbit

Reply | Threaded
Open this post in threaded view
|

Re: Feedback welcome: reworking the examples with ParameterTools

Andrea Sella
+1 for moving to external classes, it is much simpler to analyze/study few
little blocks of code than one bigger imho.

Andrea

2016-01-22 9:41 GMT+01:00 Aljoscha Krettek <[hidden email]>:

> Hi,
> the changes to the KMeans example look good so far. About moving
> everything to external classes, IMHO we should do it, but I can also see
> why it is nice to have the whole example contained in one file. So let’s
> see what the others think.
>
> Cheers,
> Aljoscha
> > On 21 Jan 2016, at 18:04, Stefano Baghino <[hidden email]>
> wrote:
> >
> > Hello dev,
> >
> > I'm currently working on [FLINK-2021] Rework examples to use
> ParameterTool
> > <https://issues.apache.org/jira/browse/FLINK-2021> and we're trying to
> > improve the readability of the K-Means example until we reach a
> > satisfactory starting point to rework the other examples as well. I
> > think Robert's
> > improvements
> > <
> https://github.com/rmetzger/flink/blob/4c3b569b522172043fe1f49da2858bae37519eef/flink-examples/flink-examples-batch/src/main/java/org/apache/flink/examples/java/clustering/KMeans.java
> >
> > (not relying on RequiredParameters and removal of static variables, in
> > particular) are good enough to be that starting point and integrated
> those
> > changes in the PR <https://github.com/apache/flink/pull/1536>.
> >
> > Before moving on I'd like to have a feedback on the work done so far.
> > Also: what would you think of moving POJOs and user functions to separate
> > files to declutter the main class? I've noticed that both in the examples
> > and in the training solutions there's a general tendency to keep
> everything
> > in one place, however my feeling is that splitting the classes would keep
> > files short and readable, while allowing quick navigation between
> > files/classes in most editors without having to scroll through the file
> to
> > reach the code you're interested in.
> >
> > --
> > BR,
> > Stefano Baghino
> >
> > Software Engineer @ Radicalbit
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Feedback welcome: reworking the examples with ParameterTools

Flavio Pompermaier
+1 as long as there's a well defined template/pattern of restructuring the
code and class-naming

On Fri, Jan 22, 2016 at 9:48 AM, Andrea Sella <[hidden email]>
wrote:

> +1 for moving to external classes, it is much simpler to analyze/study few
> little blocks of code than one bigger imho.
>
> Andrea
>
> 2016-01-22 9:41 GMT+01:00 Aljoscha Krettek <[hidden email]>:
>
> > Hi,
> > the changes to the KMeans example look good so far. About moving
> > everything to external classes, IMHO we should do it, but I can also see
> > why it is nice to have the whole example contained in one file. So let’s
> > see what the others think.
> >
> > Cheers,
> > Aljoscha
> > > On 21 Jan 2016, at 18:04, Stefano Baghino <
> [hidden email]>
> > wrote:
> > >
> > > Hello dev,
> > >
> > > I'm currently working on [FLINK-2021] Rework examples to use
> > ParameterTool
> > > <https://issues.apache.org/jira/browse/FLINK-2021> and we're trying to
> > > improve the readability of the K-Means example until we reach a
> > > satisfactory starting point to rework the other examples as well. I
> > > think Robert's
> > > improvements
> > > <
> >
> https://github.com/rmetzger/flink/blob/4c3b569b522172043fe1f49da2858bae37519eef/flink-examples/flink-examples-batch/src/main/java/org/apache/flink/examples/java/clustering/KMeans.java
> > >
> > > (not relying on RequiredParameters and removal of static variables, in
> > > particular) are good enough to be that starting point and integrated
> > those
> > > changes in the PR <https://github.com/apache/flink/pull/1536>.
> > >
> > > Before moving on I'd like to have a feedback on the work done so far.
> > > Also: what would you think of moving POJOs and user functions to
> separate
> > > files to declutter the main class? I've noticed that both in the
> examples
> > > and in the training solutions there's a general tendency to keep
> > everything
> > > in one place, however my feeling is that splitting the classes would
> keep
> > > files short and readable, while allowing quick navigation between
> > > files/classes in most editors without having to scroll through the file
> > to
> > > reach the code you're interested in.
> > >
> > > --
> > > BR,
> > > Stefano Baghino
> > >
> > > Software Engineer @ Radicalbit
> >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: Feedback welcome: reworking the examples with ParameterTools

Robert Metzger
I didn't move the classes out of the file for the following reason: People
looking at our examples might not do this with an IDE, but from Github or
the source archive.
Without an IDE, its harder to find those files. If the classes are located
just below the main class in the same file, there is no issue.

On Fri, Jan 22, 2016 at 9:58 AM, Flavio Pompermaier <[hidden email]>
wrote:

> +1 as long as there's a well defined template/pattern of restructuring the
> code and class-naming
>
> On Fri, Jan 22, 2016 at 9:48 AM, Andrea Sella <[hidden email]>
> wrote:
>
> > +1 for moving to external classes, it is much simpler to analyze/study
> few
> > little blocks of code than one bigger imho.
> >
> > Andrea
> >
> > 2016-01-22 9:41 GMT+01:00 Aljoscha Krettek <[hidden email]>:
> >
> > > Hi,
> > > the changes to the KMeans example look good so far. About moving
> > > everything to external classes, IMHO we should do it, but I can also
> see
> > > why it is nice to have the whole example contained in one file. So
> let’s
> > > see what the others think.
> > >
> > > Cheers,
> > > Aljoscha
> > > > On 21 Jan 2016, at 18:04, Stefano Baghino <
> > [hidden email]>
> > > wrote:
> > > >
> > > > Hello dev,
> > > >
> > > > I'm currently working on [FLINK-2021] Rework examples to use
> > > ParameterTool
> > > > <https://issues.apache.org/jira/browse/FLINK-2021> and we're trying
> to
> > > > improve the readability of the K-Means example until we reach a
> > > > satisfactory starting point to rework the other examples as well. I
> > > > think Robert's
> > > > improvements
> > > > <
> > >
> >
> https://github.com/rmetzger/flink/blob/4c3b569b522172043fe1f49da2858bae37519eef/flink-examples/flink-examples-batch/src/main/java/org/apache/flink/examples/java/clustering/KMeans.java
> > > >
> > > > (not relying on RequiredParameters and removal of static variables,
> in
> > > > particular) are good enough to be that starting point and integrated
> > > those
> > > > changes in the PR <https://github.com/apache/flink/pull/1536>.
> > > >
> > > > Before moving on I'd like to have a feedback on the work done so far.
> > > > Also: what would you think of moving POJOs and user functions to
> > separate
> > > > files to declutter the main class? I've noticed that both in the
> > examples
> > > > and in the training solutions there's a general tendency to keep
> > > everything
> > > > in one place, however my feeling is that splitting the classes would
> > keep
> > > > files short and readable, while allowing quick navigation between
> > > > files/classes in most editors without having to scroll through the
> file
> > > to
> > > > reach the code you're interested in.
> > > >
> > > > --
> > > > BR,
> > > > Stefano Baghino
> > > >
> > > > Software Engineer @ Radicalbit
> > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: Feedback welcome: reworking the examples with ParameterTools

stefanobaghino
Thanks for the insight, I haven't thought about it.

On Fri, Jan 22, 2016 at 10:10 AM, Robert Metzger <[hidden email]>
wrote:

> I didn't move the classes out of the file for the following reason: People
> looking at our examples might not do this with an IDE, but from Github or
> the source archive.
> Without an IDE, its harder to find those files. If the classes are located
> just below the main class in the same file, there is no issue.
>
> On Fri, Jan 22, 2016 at 9:58 AM, Flavio Pompermaier <[hidden email]>
> wrote:
>
> > +1 as long as there's a well defined template/pattern of restructuring
> the
> > code and class-naming
> >
> > On Fri, Jan 22, 2016 at 9:48 AM, Andrea Sella <
> [hidden email]>
> > wrote:
> >
> > > +1 for moving to external classes, it is much simpler to analyze/study
> > few
> > > little blocks of code than one bigger imho.
> > >
> > > Andrea
> > >
> > > 2016-01-22 9:41 GMT+01:00 Aljoscha Krettek <[hidden email]>:
> > >
> > > > Hi,
> > > > the changes to the KMeans example look good so far. About moving
> > > > everything to external classes, IMHO we should do it, but I can also
> > see
> > > > why it is nice to have the whole example contained in one file. So
> > let’s
> > > > see what the others think.
> > > >
> > > > Cheers,
> > > > Aljoscha
> > > > > On 21 Jan 2016, at 18:04, Stefano Baghino <
> > > [hidden email]>
> > > > wrote:
> > > > >
> > > > > Hello dev,
> > > > >
> > > > > I'm currently working on [FLINK-2021] Rework examples to use
> > > > ParameterTool
> > > > > <https://issues.apache.org/jira/browse/FLINK-2021> and we're
> trying
> > to
> > > > > improve the readability of the K-Means example until we reach a
> > > > > satisfactory starting point to rework the other examples as well. I
> > > > > think Robert's
> > > > > improvements
> > > > > <
> > > >
> > >
> >
> https://github.com/rmetzger/flink/blob/4c3b569b522172043fe1f49da2858bae37519eef/flink-examples/flink-examples-batch/src/main/java/org/apache/flink/examples/java/clustering/KMeans.java
> > > > >
> > > > > (not relying on RequiredParameters and removal of static variables,
> > in
> > > > > particular) are good enough to be that starting point and
> integrated
> > > > those
> > > > > changes in the PR <https://github.com/apache/flink/pull/1536>.
> > > > >
> > > > > Before moving on I'd like to have a feedback on the work done so
> far.
> > > > > Also: what would you think of moving POJOs and user functions to
> > > separate
> > > > > files to declutter the main class? I've noticed that both in the
> > > examples
> > > > > and in the training solutions there's a general tendency to keep
> > > > everything
> > > > > in one place, however my feeling is that splitting the classes
> would
> > > keep
> > > > > files short and readable, while allowing quick navigation between
> > > > > files/classes in most editors without having to scroll through the
> > file
> > > > to
> > > > > reach the code you're interested in.
> > > > >
> > > > > --
> > > > > BR,
> > > > > Stefano Baghino
> > > > >
> > > > > Software Engineer @ Radicalbit
> > > >
> > > >
> > >
> >
>



--
BR,
Stefano Baghino

Software Engineer @ Radicalbit
Reply | Threaded
Open this post in threaded view
|

AW: Feedback welcome: reworking the examples with ParameterTools

Fabian Hueske-2
Hi,
I agree with Robert on this.

We tried to keep the examples concise and moved boilerplate code like the parameter parsing to the end of the file for that reason. I’m +1 for removing as much boilerplate code as possible but would prefer to keep the example selfcontained and in one file.

Best, Fabian






Von: Stefano Baghino
Gesendet: Freitag, 22. Januar 2016 10:14
An: [hidden email]
Betreff: Re: Feedback welcome: reworking the examples with ParameterTools

Thanks for the insight, I haven't thought about it.

On Fri, Jan 22, 2016 at 10:10 AM, Robert Metzger <[hidden email]>
wrote:

> I didn't move the classes out of the file for the following reason: People
> looking at our examples might not do this with an IDE, but from Github or
> the source archive.
> Without an IDE, its harder to find those files. If the classes are located
> just below the main class in the same file, there is no issue.
>
> On Fri, Jan 22, 2016 at 9:58 AM, Flavio Pompermaier <[hidden email]>
> wrote:
>
> > +1 as long as there's a well defined template/pattern of restructuring
> the
> > code and class-naming
> >
> > On Fri, Jan 22, 2016 at 9:48 AM, Andrea Sella <
> [hidden email]>
> > wrote:
> >
> > > +1 for moving to external classes, it is much simpler to analyze/study
> > few
> > > little blocks of code than one bigger imho.
> > >
> > > Andrea
> > >
> > > 2016-01-22 9:41 GMT+01:00 Aljoscha Krettek <[hidden email]>:
> > >
> > > > Hi,
> > > > the changes to the KMeans example look good so far. About moving
> > > > everything to external classes, IMHO we should do it, but I can also
> > see
> > > > why it is nice to have the whole example contained in one file. So
> > let’s
> > > > see what the others think.
> > > >
> > > > Cheers,
> > > > Aljoscha
> > > > > On 21 Jan 2016, at 18:04, Stefano Baghino <
> > > [hidden email]>
> > > > wrote:
> > > > >
> > > > > Hello dev,
> > > > >
> > > > > I'm currently working on [FLINK-2021] Rework examples to use
> > > > ParameterTool
> > > > > <https://issues.apache.org/jira/browse/FLINK-2021> and we're
> trying
> > to
> > > > > improve the readability of the K-Means example until we reach a
> > > > > satisfactory starting point to rework the other examples as well. I
> > > > > think Robert's
> > > > > improvements
> > > > > <
> > > >
> > >
> >
> https://github.com/rmetzger/flink/blob/4c3b569b522172043fe1f49da2858bae37519eef/flink-examples/flink-examples-batch/src/main/java/org/apache/flink/examples/java/clustering/KMeans.java
> > > > >
> > > > > (not relying on RequiredParameters and removal of static variables,
> > in
> > > > > particular) are good enough to be that starting point and
> integrated
> > > > those
> > > > > changes in the PR <https://github.com/apache/flink/pull/1536>.
> > > > >
> > > > > Before moving on I'd like to have a feedback on the work done so
> far.
> > > > > Also: what would you think of moving POJOs and user functions to
> > > separate
> > > > > files to declutter the main class? I've noticed that both in the
> > > examples
> > > > > and in the training solutions there's a general tendency to keep
> > > > everything
> > > > > in one place, however my feeling is that splitting the classes
> would
> > > keep
> > > > > files short and readable, while allowing quick navigation between
> > > > > files/classes in most editors without having to scroll through the
> > file
> > > > to
> > > > > reach the code you're interested in.
> > > > >
> > > > > --
> > > > > BR,
> > > > > Stefano Baghino
> > > > >
> > > > > Software Engineer @ Radicalbit
> > > >
> > > >
> > >
> >
>



--
BR,
Stefano Baghino

Software Engineer @ Radicalbit