Skip to content
This repository has been archived by the owner on Dec 11, 2018. It is now read-only.

Refactored and added discarding logic #45

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mery25
Copy link

@mery25 mery25 commented Oct 3, 2018

  • Separated Exception Handling
  • Separate persistence logic to a TweetRepository
  • Created PatternExtractor to extract links from Tweet
  • Added new endpoints to discard and get discarted tweets. Added logic on Controller, Service and Repository
  • Added fields needed on the Tweet entity and created a new Link entity
  • Created new test cases

…ed a PatternExtractor to extract the links from the tweet. Added validation annotation on the Tweet fiels to use the Spring VAlidator to validate the fields. Added validation logic on the TweetService. Added new test cases.
…sitory from the TweetService. Solved some bugs. Added some test cases.
@LordKhizir
Copy link
Collaborator

LordKhizir commented Oct 4, 2018

(Prospect for FOTOCASA)
Hi Maria,
thanks for your time & effort!
We'll review this solution and get back to you.

Whether OK or KO, we'll provide you with constructive feedback.

See ya!

m.appendReplacement(sb, "");
}
m.appendTail(sb);
this.text = sb.toString();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, is it a bit strange to let the user of this class the responsability of knowing that he/she has to call the extract method, right? What do you think? How could you do it differently so it is easy to use and the results are inmutable?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other words, if text and pattern are always the same, so the extract method always will return the same, what is the point of letting people call it more than once?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converted PatternExtractor in a BiFunction that takes a two String and return a PatternExtractorResult

}

private String insertLinksOnTweet(String text, List<Link> links) {
if (links.size() == 0)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is a bit complex, is it unit tested? Do you think should be in another class maybe?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied same solution than for the PatternExtractor. Created a TextElementInsterter, which is a BiFunction that takes a list of TextElement and a String and returns a String. I have created test cases for TextElementInserter

…racted from Tweet class. Created TextElement and made Link implement from it. Created test cases for TextElementInsterter
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants