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 |
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 |
+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 > > |
+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 > > > > > |
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 > > > > > > > > > |
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 |
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 |
Free forum by Nabble | Edit this page |