Github user ilganeli commented on the pull request:
https://github.com/apache/spark/pull/5396#issuecomment-90747691
Mark - you've convinced me. With that said, do you see any way to make the
code more modular without sacrificing the protection from scoping? My primary
intent with my refactoring was to break things out into discrete function
calls. Would moving those internal to where they're used help?
Sent with Good (www.good.com)
-----Original Message-----
From: Mark Hamstra
[[email protected]<mailto:[email protected]>]
Sent: Tuesday, April 07, 2015 05:55 PM Eastern Standard Time
To: apache/spark
Cc: Ganelin, Ilya
Subject: Re: [spark] [SPARK-6746B] Refactor large functions in DAGScheduler
to improve readibility (#5396)
I guess I just don't see things the same way. I don't have a problem with
the DAGScheduler being implemented with scoping idioms that may be unfamiliar
to a large number of coders. The DAGScheduler is about as far from being a
public API in Spark as you can get, and I don't really think we want to
sacrifice the additional safety that nested scopes provide within the
DAGScheduler just to make it easy for more developers to make changes to that
part of Spark's code. If you want an even more contrary point of view, a high
bar of expected familiarity with certain idioms actually can serve us well in
restricting just who is making changes to the DAGScheduler and what style and
concerns they are maintaining.
Quite simply, I don't find a function declared within the scope of another
function, for example, to be at all difficult to read -- it's just a common
idiom across several programming languages with which I am familiar. On the
other hand, flattening out those carefully nested scopes does actually make it
harder for me to read and reason about where and under what preconditions a
previously-nested function can and should be used.
I'm not saying that I won't consider any refactoring of the DAGScheduler or
the flattening of any scopes; but in order for me not to veto them, any such
changes will require a lot more justification than simply that making them
produces code that is more readable for a larger audience of developers.
â
Reply to this email directly or view it on
GitHub<https://github.com/apache/spark/pull/5396#issuecomment-90743681>.
________________________________________________________
The information contained in this e-mail is confidential and/or proprietary
to Capital One and/or its affiliates. The information transmitted herewith is
intended only for use by the individual or entity to which it is addressed. If
the reader of this message is not the intended recipient, you are hereby
notified that any review, retransmission, dissemination, distribution, copying
or other use of, or taking of any action in reliance upon this information is
strictly prohibited. If you have received this communication in error, please
contact the sender and delete the material from your computer.
---
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]
