Port of Spruce #12

bret-fears · 2017-02-27T20:26:24Z

Port of SortFunction:

Create Spruce main class with builder pattern API
Add base class for SortFunction to be extended

Refactor:

provide a builder pattern that takes a view group with the option of adding a sort function and an animator
Add java docs and comments
Remove redundant interfaces

evant

Might be better to come together and whiteboard an api design.

evant · 2017-02-27T22:18:46Z

lib/src/main/java/com/willowtreeapps/spruce/SortFunctions/SortFunction.java

+/**
+ * SpruceView
+ * <p>
+ * Copyright (c) 2017 WillowTree, Inc.


Should go at top of file. You can create a Copyright Profile in AS to auto-add these.

I was trying to do that under File and Code Templates . I've added a Spruce copyright that now does the right thing. Thanks for calling that out.

evant · 2017-02-27T22:20:14Z

lib/src/main/java/com/willowtreeapps/spruce/SortFunctions/SortFunctionImpl.java

+ * THE SOFTWARE.
+ **/
+
+public interface SortFunctionImpl {


Don't really get the impl distinction, why not just have the abstract class?

I'm also curious how often this recursive depth param comes up in practice, it will really complicate the implementations.

My initial thought was to use the Impl as a sort of class schema but I can do without it.

evant · 2017-02-27T22:21:10Z

lib/src/main/java/com/willowtreeapps/spruce/SpruceView.java

+ * THE SOFTWARE.
+ **/
+
+public class SpruceView extends View implements SpruceViewImpl {


Do we really need to subclass a specific view? That's going to be really hard to do in practice.

tylerjromeo

I think these view could definitely use javadocs and usage examples because I'm having trouble telling from the outside how to use a "SpruceView" or what it is supposed to do

tylerjromeo · 2017-02-28T15:20:43Z

.idea/encodings.xml

-  <component name="Encoding">
-    <file url="PROJECT" charset="UTF-8" />
-  </component>
-</project>


good to remove this, but is .idea in your gitignore?

it is in the gitignore as well. not sure how they still ended up getting added

tylerjromeo · 2017-02-28T15:22:00Z

lib/src/main/java/com/willowtreeapps/spruce/SortFunctions/SortFunctionImpl.java

+
+import java.util.List;
+
+public interface SortFunctionImpl {


the Impl suffix usually implies that this is the implementation in java. If you want this to be the interface I'd use ISortFunction

tylerjromeo · 2017-02-28T15:23:18Z

lib/src/main/java/com/willowtreeapps/spruce/SortFunctions/SortFunction.java

+ *     OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ *     THE SOFTWARE.
+ *
+ */


Isn't this usually in like... a LICENSE.txt instead of every file? If you want to change the LICENSE this is gonna be a HUGE pain

It is. @jacksontaylor13 has it as headers for his files as well so I was copying him. You're right though, changing many is far more difficult than changing one.

@tylerjromeo @bret-fears I was told by biz-dev that when releasing this publicly we would need to add the license to the header of each document. This is something that Apple has always done and a lot of open source iOS projects.

tylerjromeo · 2017-02-28T15:23:30Z

lib/src/main/java/com/willowtreeapps/spruce/SortFunctions/SortFunction.java

+public abstract class SortFunction implements SortFunctionImpl {
+
+    public List<SpruceTimedView> getTimeOffsets(SpruceView view) {
+        return getTimeOffsets(view, /*recursiveDepth=*/0);


cool named param

I can't take credit for that. I saw @evant doing it before and adopted it since. I think he got it from AOSP

tylerjromeo · 2017-02-28T15:23:53Z

lib/src/main/java/com/willowtreeapps/spruce/SortFunctions/SortFunction.java

+
+import java.util.List;
+
+public abstract class SortFunction implements SortFunctionImpl {


Why use an abstract class with no abstract functions?

bret-fears · 2017-03-01T14:14:34Z

I git rm --cached -r .idea on the last commit

evant · 2017-03-01T14:44:06Z

lib/src/main/java/com/willowtreeapps/spruce/SortFunctions/SortFunction.java

+     * @param timeOffset (long) time offset in milliseconds
+     * @return List of SpruceTimedView objects that contain the view and it's offset
+     */
+    public abstract List<SpruceTimedView> getTimeOffsets(View view, long timeOffset);


Shouldn't this take a ViewGroup since it's returning a list of it's children?

Yes, that's a good point.

Port of SortFunction and it's dependency of SpruceView -Create Spruce main class with build pattern API -Add base class for SortFunction to be extended Refactor - provide a builder pattern that takes a view group with the option of adding a sort function and an animator - Add java docs and comments - Remove redundant interfaces

bret-fears self-assigned this Feb 27, 2017

bret-fears requested review from tylerjromeo and evant February 27, 2017 20:26

evant reviewed Feb 27, 2017

View reviewed changes

tylerjromeo reviewed Feb 28, 2017

View reviewed changes

bret-fears force-pushed the sort-function branch 2 times, most recently from ab4d7d0 to 35f0883 Compare February 28, 2017 20:30

bret-fears changed the title ~~Port of SpruceView and pairing interface~~ Port of SpruceView Feb 28, 2017

bret-fears force-pushed the sort-function branch 3 times, most recently from e338129 to d1f984e Compare March 1, 2017 14:12

bret-fears force-pushed the sort-function branch 2 times, most recently from 42db7bf to 9dedc97 Compare March 1, 2017 14:27

evant reviewed Mar 1, 2017

View reviewed changes

bret-fears force-pushed the sort-function branch from 64d9ea8 to eb7c659 Compare March 1, 2017 15:25

bret-fears changed the title ~~Port of SpruceView~~ Port of Spruce Mar 1, 2017

bret-fears merged commit 52d9625 into develop Mar 1, 2017

bret-fears deleted the sort-function branch March 3, 2017 18:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port of Spruce #12

Port of Spruce #12

bret-fears commented Feb 27, 2017 •

edited

Loading

evant left a comment

evant Feb 27, 2017

bret-fears Feb 28, 2017

evant Feb 27, 2017

bret-fears Feb 28, 2017

evant Feb 27, 2017

tylerjromeo left a comment

tylerjromeo Feb 28, 2017

bret-fears Feb 28, 2017

tylerjromeo Feb 28, 2017

tylerjromeo Feb 28, 2017

bret-fears Feb 28, 2017

jacksontaylor13 Mar 1, 2017

tylerjromeo Feb 28, 2017

bret-fears Feb 28, 2017

tylerjromeo Feb 28, 2017

bret-fears commented Mar 1, 2017

evant Mar 1, 2017

bret-fears Mar 1, 2017


		import java.util.List;

		public abstract class SortFunction implements SortFunctionImpl {

Port of Spruce #12

Port of Spruce #12

Conversation

bret-fears commented Feb 27, 2017 • edited Loading

evant left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tylerjromeo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bret-fears commented Mar 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bret-fears commented Feb 27, 2017 •

edited

Loading