The Top Stories apparently can include job postings. I have the
StoryAdapter only accepting responses that have the type of "story". We need to build up a path for
Jobs because it's in the payload. Well... mostly because it crashes the app when I scroll.
We can assume that we'll have a
Job class like we have a
Gonna swing in the test class to TDD it up. looks around Still not paired.
Job is going to be VERY similar to
Story; I'll just C&P the tests and classes around - I know; Boo. But it's quicker. C&P means an abstraction is missing.
I expect that I'll end up back at "item" types. At least as a base class.
The first class I'm C&Ping over is
StoryId. This really should be abstracted.
JobId will be empty classes... and we don't need that. Which means that
ItemId will have a
createJobId which will use slugs as identifiers... yea... I guess so...
Then add a
This has turned into less of a C&P and more of a C&P means abstraction; so abstraction.
OK; ONTO THE CHANGES!
I did a rename of
ItemId. Added a
JOB_ID slug; and the instance var to hold it.
Privatized the constructor and replaced all constructions with
createStoryId which provides the slug to the private constructor.
NULL_ITEM_ID has a null slug.
Some string updates and all 119 tests pass!
We need to rename the
ItemIdTests and TDD up some checks for the
Which now works wonderfully. I used the
NULL_ITEM_ID to validate that
We now can create
isJobId. This will help drive the rest of the Job related classes.
Looking at the
Story class and the hacker news api doc; only the ID is a common field; I don't know if that's enough to create an abstraction. MAYBE? If it shares enough functionality, it would make sense. I'm not going to force it right now; just slowly build out the Job related classes like I did for the Story.
The doc's have ID as the only REQUIRED... all items appear to have more commonality.
"by" : "dhouston", "id" : 8863, "time" : 1175714200, "type" : "story|job|..."
This is enough that an abstraction is warranted.
Item pulled out;
Story now extends
I'm kinda waffling on if
score should be in
Item. It makes sense right now as it's shared between
Job... and they'll have duplicate code...
Red - Green - Refactor.
I'm trying to pre-optimize. That's a no-no. I'm going to leave
Job in; and empty!!! There will be a "URL" field. Which
Story will also have, so the thought still applies. Right now I'm trying to avoid a crash when a job comes in, let's get that working.
There's been a significant chunk of refactoring around creating the required Job classes. Now to test it!
... Well Crap... ish. There's a Job coming back in the top 20 or so... which is crashing the app... :\ hehe
This isn't working
SIGH So; there needs to be a much heavier refactor of
Item. I can probably hack some stuff in; but.... Noooo.... I'll have to set aside a few hours to heavily refactor so that the
StoryAdapter is an
ItemAdapter. This is pretty much required for what I want to accomplish. ... yay.
Until I get the time to hammer at that - ... I would ask you to wait... but this is a blog; you'll just continue reading the result when I type it out. Lucky you!
I really enjoy the irony that I started with
Item classes and changed to
Story and am now having to go back to
Item. Which I fully support because we didn't know we'd need it.
Continuing the trend of not distinguishing between Story and Job data - The
HackerNewsNetwork class with the separate
job method... Nope. Need a single one.
There's a big dance of refactoring going on here... It's kinda crufty to see what needs to be done; but I expect beauty when it's complete.
I wrote a test to test the
TopStoryAdapter#onBindViewHolder to bind a job. Which will need some work as there is slight variation between a story and a job ui.
This test is what's driving a lot of other testing. I've commented out the driving test; and am ensuring I fix each failing test as I make changes.
This is a time when running tests like crazy is needed. The smallest step; make sure it works.
AHHHHH! 7 FAILING TESTS!!! UNDO UNDO UNDO UNDO UNDO UNDO UNDO
I changed two things. Bad me.
I was struggling along for a while; hunting down the code I missed in the story->item migration. I had 5 failing tests.
Finally traced it down to a section checking specifically for story id's. Not sure how it was missed originally - And BAM! all tests work.
Now to go re-enable the original driver test. And run it to make sure it still fails. Yep; fails. Imagine that - It doesn't display the job correctly... since I haven't implemented it!
One thing that the code keeps YELLING at me about is that
Story should be encapsulated in
Item. I keep getting directed to add a
Item. Base classes should not know about their derived types; so this doesn't work out well.
I think I've gotten past the little hiccup I was having about migrating it in. I was trying to take a big bite; now I'm doing a lot of small stuff.
I put in
create methods that just called the
Then built up all the checks around while having it go
I really hope that this process of tearing out the
Story class doesn't backfire into painful test failures... And there's just 1 failure! Which is a string that got overly refactored in the test project.
I now have no
Job class. All functionality still exists... Well; tests still pass.
A huge boon of having the Adapter covered in tests; I /know/ that it will work for either jobs or stories. I have tests that show it.
There's a little bit of renaming to do to eliminate the reference to
Stories; but, small detail.
I now need to run the app in an emulator to just confirm that it's working right.
I should really write a UI test to check this for me... But it's 1am; so... It gets shoved to the back.
All told; the refactor once I sat down took about 3, maybe 4 hours. What I expected. My test count ... went up 5. Neat; and class count went down by 2.
Running this on the emulator... apparently some files aren't closing...
E/NativeCrypto: AppData::create pipe(2) failed: Too many open files
Ahh - Yes. Network. I'm spinning up a new
OkHttpClient for each request. I'll have to optimize how the OKHttp client is handled. I suspected I would early on; but now I know; and now is the correct time to do it.
less optimize; more... not be broken.
Doing it "now" being, later.
I want to highlight that I would have spent WAY more than 4 hours trying to remove the
Job classes. To get them into the
Item class. I probably would have never done it. Just hacked in
ifs to see what type it is.
The work I'm doing here exemplifies why the XP practices make coding better. I am reducing the class count; increasing the test coverage and simplifying the code just by applying good object oriented designs.
It shows that having solid test coverage allows focus to remain on feature work and not trying to verify functionality.
Did you pick up on the lack of modification to the UI to support displaying Jobs?
I could have just left out the changes, sure; I didn't. There were no changes to support displaying Jobs. I expected there to be. My driving test was around the adapter to see jobs displayed. Once everything was an item. BAM! worked. Using a lot of Object classes and the Null Object pattern allowed customization to the outputs in those; instead of hacking them into, and requiring,
The result; It. Displays. It came out far cleaner than I expected. I'm thrilled the practices are showing their immense value.