Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/14332#discussion_r71994171
--- Diff:
examples/src/main/scala/org/apache/spark/examples/ml/DataFrameExample.scala ---
@@ -54,14 +54,13 @@ object DataFrameExample {
}
}
- parser.parse(args, defaultParams).map { params =>
- run(params)
- }.getOrElse {
- sys.exit(1)
+ parser.parse(args, defaultParams) match {
+ case Some(params) => run(params)
+ case _ => sys.exit(1)
}
--- End diff --
There's nothing wrong with an expression of type `Unit` per se, but it can
only provide side effects. `map` defines a transformation, but such an
expression doesn't transform into anything. Further, when the transformations
(thus side effects) occur varies. If the primary purpose is to trigger side
effects, right now, we have `foreach` right? That's what's going on in the rest
of the change. I think that's uncontroversial.
How about an argument from consistency? The other changes exist because of
a potential correctness problem. I don't _think_ the same problem can ever
happen with `Option` because of `getOrElse`. But it seems reasonable to avoid
the same construct anyway, per argument above, and per consistency. The
question is, what to replace it with?
`if-else` or `match` both seem fine and straightforward to me. Both are
also at least as "obvious" to me as `map` or `fold`, probably moreso. From an
FP perspective, are those actually appropriate with side-effect-only
expressions. At least, why would those be a better choice than something built
around `foreach`?
Yeah I don't think `fold` can have the same problem actually since it would
always force evaluation, scratch that. Or else, it seems to in a test like the
`map` one in the JIRA. It's viable as a way to trigger side effects, it seems,
even if I'd say it's still not great FP.
I personally would not use it as an alternative for handling `Option`
because the semantics of `fold` are about combining multiple values into one.
However this usage isn't combining anything, and there aren't even values
except trivially `Unit`. Is it obvious in `option.fold(foo)(bar)` that `foo`
happens if it's _not_ defined and `bar` happens when it is? compared to
`if-else` or `match`, where it's entirely explicit?
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
