Architektura pająka internetowego

0

Mam problem jak dzielić program na klasy/interfejsy/pakiety tak by Wujek Bob nie krzyczał, że łamię zasadę Open Close. (klasy otwarte na rozszerzenie ale zamkniete na zmiany).

Poniżej kod części programu: klasa Topic to wątek na forum, a Page to strona. Chcę je tak zaprojektować by łatwo można było się przestawić na inne forum, a klasa która decyduje na jakim forum praucjemy to klasa implementująca PageIterator. Zadaniem tego programiku jest iteracja przez strony wskazanego wątku i tworzenie na nich obiektu Page.
Możecie podpowiedzieć czy tak to ma być?

// example of use:
public class AppMain{
public static void main(String[] args){
                String url = "https://4programmers.net/Forum/Newbie/210891-od_czego_zaczac_nauke_programowania";
		Topic topic = new Topic(url);
	
		PageIterator it = topic.iterator();
		//it.next();
		Page page = it.next();
}
}

/*
 * to consider: what if the view of the post will be changed from 10 to 50?
 */
public class Topic implements Iterable<Page> {

	private String urlBase;    // URL of the topic on forum
	private String pagePattern; // pattern whcich change while changing page number of the topic, for example: ?page=
	private PageIteratorEnum pageIteratorType; // type of the forum 4programmers? Codercity? etc.

	// to consider: put these variables into abstract class?

	public Topic(String url, String pagePattern) {
		this.urlBase = url;
		this.pagePattern = pagePattern;
	}

	public Topic(String url) {
		this.urlBase = url;
		this.pagePattern = "?page=";
	}

	public void setUrlBase(String html) {
		this.urlBase = html;
	}

	public String getUrlBase() {
		return urlBase;
	}

	public void setPagePattern(String pattern) {
		pagePattern = pattern;
	}

	public String getPagePattern() {
		return pagePattern;
	}

	public String toString() {
		return getUrlBase();
	}

	public void setIterator(PageIteratorEnum type){
		pageIteratorType = type;
	}

	@Override
	public PageIterator iterator() {
		if(pageIteratorType == null){
			throw new RuntimeException("First set iterator type by setIterator() method.");
		}
		return new PageIteratorFactoryImp1(this).produceIterator(pageIteratorType);
	}
}

...

import org.jsoup.nodes.Document;

public class Page {

	Document doc;
	
	public Page(Document doc){
		this.doc = doc;
	}
}

...

import java.util.Iterator;

public interface PageIterator extends Iterator<Page>{

	public boolean hasNext();
	
	public Page next();
}

...

public enum PageIteratorEnum {

	it4PROGRAMMERS, itCODERCITY;
}

...

public interface PageIteratorFactory {

	public PageIterator produceIterator(PageIteratorEnum type);
}

...

public class PageIteratorFactoryImp1 implements PageIteratorFactory{

	Topic topic;
	
	public PageIteratorFactoryImp1(Topic topic){
		this.topic = topic;
	}
	
	public PageIterator produceIterator(PageIteratorEnum type){
		if(type.equals("it4PROGRAMMERS")){
			return new TopicPageIterator4programmers(topic);
		}
		return null;
	}
}

...

import java.io.IOException;

import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;

public class TopicPageIterator4programmers implements PageIterator {

	private Topic topic;
	private int count = 0;
	String urlBase;
	String pagePattern;
	String browser = "Mozilla/5.0";
	NavigatePanel navigation;

	public TopicPageIterator4programmers(Topic topic) {
		this.topic = topic;
		this.urlBase = topic.getUrlBase();
		this.pagePattern = topic.getPagePattern();
		this.navigation = new NavigatePanel4programmers(urlBase, pagePattern);
	}

	@Override
	public boolean hasNext() {
		return ((count + 1) <= navigation.getMaxPageNumber());
	}

	@Override
	public Page next() {
		Page result = null;
		String newUrl = urlBase + pagePattern + String.valueOf(++count);
		Document doc = null;
		try {
			doc = Jsoup.connect(newUrl).userAgent(browser).get();
			result = new Page4programmers(doc, count);
		} catch (IOException e) {
			System.out.println("something went wrong with downloading the page: " + newUrl);
			e.printStackTrace();
		}
		return result;

	}

}
1

Po pierwsze zbędny dla ciebie jest enum PageIteratorEnum. W klasie topic nigdzie go nie ustawiasz (jest setter, ale nie ustawiasz żadnej wartości w przykładowym użyciu, bo ty było bez sensu. U ciebie to jaką dostaniesz implementację decyduje fabryka na podstawie enuma, który najpierw musisz ustawić w klasie topic - skoro wiesz jaki enum ustawić to prawdopodobnie wiesz jakiej implementacji użyć ;). Generalnie jeżeli byś chciał dodać obsługę nowego typu forum to musisz dodać nową implementacje PageIteratora, nowy typ w ww enumie i zmienić implementację fabryki - więc potrzebujesz dodać nową klasę i zmienić 2 istniejące, moim zdaniem to jest złamanie OCP. Idealnym rozwiązaniem byłoby gdybyś tylko musiał dodać nowy typ PageIteratora, ale potrzebujesz dodatkowo fabryki, która dostarczy konkretną implementację. Byłoby to możliwe gdyby np. fabryka poprzez refleksję znalazła wszystkie klasy implementujące PageIterator, a implementacje PageIteratora zawierałyby metodę, która decyduje czy dana klasa potrafi obsłużyć twój topic. np

public boolean canHandle(Topic topic) {
   //logika
}

Mógłbyś też zamiast korzystać z refleksji użyć implementacji fabryki (interfejsu nigdzie nie wykorzystujesz) i modyfikowac fabrykę za każdym razem gdy dodajesz nowy typ PageIteratora tak by fabryka go obsługiwała (podobnie jak przy uzyciu refleksji tylko sam musialbyś dodawać kolejne typy PageIteratora do jakiejś kolekcji). Fabryka wtedy będzie łamać OCP (ale PageIterator nie), bo dodając nowy typ musisz ją zmodyfikować. Zależy jak wielkim purystą chcesz być, ale może ktoś ma lepsze rozwiązanie

1

To mi wygląda na słaby punkt:

public class TopicPageIterator4programmers implements PageIterator {
[...]
    @Override
    public PageIterator iterator() {
        if(pageIteratorType == null){
            throw new RuntimeException("First set iterator type by setIterator() method.");
        }
        return new PageIteratorFactoryImp1(this).produceIterator(pageIteratorType);
    }
}

Bardziej elegancko byłoby zaprojektować ten Topic tak, żeby nie był zależny od jakiejś tymczasowej implementacji (PageIteratorFactoryImp1). Czyli albo tworzyć go fabryką albo dawać mu tę fabrykę w konstruktorze. Jak najmniej new.

Może tak:

ForumFactory fact = ForumFactory.createFactory(); //
fact.createTopic(url); // obiekt Topic dostanie na dzień dobry od fabryki to,
                       // czego mu trzeba do dalszych wywołań, np. PageIteratorFactory

Te wszystkie factory byłyby warstwą konfiguracyjną, którą zmienialibyśmy w miarę potrzeb. Reszta byłaby stała.

No i setIterator. To jakaś niedokończona koncepcja. Może ForumFactory rozwiązuje ten problem. Takie stałe elementy inicjalizacyjne powinny iść przez konstruktor, nie przez właściwość.

0

A co powiecie jakby to przerobić tak o:

import java.io.IOException;

public class AppMain{

	public static void main(String[] args) throws IOException {
		TopicFactory tf = new TopicFactoryV1();
		Topic t = tf.produceTopic("https://4programmers.net/Forum/Newbie/210891-od_czego_zaczac_nauke_programowania");
		int i = 0;
		for (Page p : t) {
			i++;
		}
		System.out.println("zliczyłem " + i + " stron.");
	}
}
public interface TopicFactory {

	public Topic produceTopic(String url);
}
public class TopicFactoryV1 implements TopicFactory {

	@Override
	public Topic produceTopic(String url) {
		String tmp = url.split("/")[2];
		if (tmp.equals(ForumType.PASJA_INFORMATYKI.getHttp())) {
			// return new TopicPasjaInformatyki(url);
		} else if (tmp.equals(ForumType._4PROGRAMMERS.getHttp())) {
			return new Topic4programmers(url);
		}
		throw new RuntimeException("Chosen URL is undefined. Donno how to crawl through it.");
	}

}

public enum ForumType {

	_4PROGRAMMERS("4programmers.net"), PASJA_INFORMATYKI("forum.pasja-informatyki.pl");

	private String http;

	private ForumType(String http){
		this.http = http;
	}
	
	public String getHttp(){
		return http;
	}
}
package _4programmers;

import java.io.IOException;
import java.util.Iterator;

import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;

import Abstract.Page;
import Abstract.Topic;

/*
 * Using lazy loading becuz downloading the page cost much time and 
 *     from one startPage there can be generated a few pages.
 * What is more getPage() method doesn't need to initialize navigatePanel and startPage.

 * to consider: what if the view of the post will be changed from 10 to 50?

 */
public class Topic4programmers extends Topic implements Iterable<Page>{

	// to consider: put these variables into abstract class?
	private String urlBase;
	private String pagePattern;
	private NavigatePanel4programmers navigatePanel;
	private final String BROWSER = "Mozilla/5.0";

	public Topic4programmers(String url, String pagePattern) {
		this.urlBase = url;
		this.pagePattern = pagePattern;
	}

	public Topic4programmers(String url) {
		super.urlBase = url;
		this.pagePattern = "?page=";
	}

	private void initNavigatePanel() {
		navigatePanel = new NavigatePanel4programmers(urlBase, BROWSER);
	}

	public int getMaxPageNumber() {
		if (navigatePanel == null) {
			initNavigatePanel();
		}
		return navigatePanel.getMaxPageNumber();
	}

	public Page getPage(int pageNumber) {
		Page result = null;
		String newUrl = urlBase + pagePattern + String.valueOf(pageNumber);
		Document doc = null;
		try {
			doc = Jsoup.connect(newUrl).userAgent("Mozilla/5.0").get();
			result = new Page4programmers(doc, pageNumber);
			// System.out.println(doc);
		} catch (IOException e) {
			System.out.println("something went wrong with downloading the page: " + newUrl);
			e.printStackTrace();
		}
		return result;
	}

	public void setUrlBase(String html) {
		this.urlBase = html;
		// set to null cuz it must be initialize after changing the urlBase.
		navigatePanel = null;
	}

	public String getUrlBase() {
		return urlBase;
	}

	@Override
	public void setPagePattern(String pattern) {
		pagePattern = pattern;
	}

	@Override
	public String getPagePattern() {
		return pagePattern;
	}

	public NavigatePanel4programmers getNavigatePanel() {
		if (navigatePanel == null) {
			initNavigatePanel();
		}
		return navigatePanel;
	}

	@Override
	public String toString() {
		return getUrlBase();
	}

	@Override
	public Iterator<Page> iterator() {
		return this.new Iterator4programmers();
	}

	private class Iterator4programmers implements Iterator<Page> {

		private int count = 0;

		@Override
		public boolean hasNext() {
			return ((count + 1) <= Topic4programmers.this.getMaxPageNumber());
		}

		@Override
		public Page next() {
			return Topic4programmers.this.getPage(++count);
		}

	}

}
public abstract class Page{

}
public Page4programmers extends Page{

}
0

Jak już się uparłeś na tego enuma to polecałbym olać fabrykę i w enumie dodać statyczną metodę fabrykującą - wtedy zmiany, które będziesz musiał wprowadzić wraz z nowym typem forum ograniczysz do 2 klas

edit:
https://pastebin.com/6JhF5w6y
Więc zamiast fabryki wołasz tylko ForumType.fromDomain(). Metodę canHandle zrobiłem abstrakcyjną gdyby tam miała być różna logika, ale póki co jest taka sama w obydwóch typach, wiec nie musi być abstrakcyjną

1 użytkowników online, w tym zalogowanych: 0, gości: 1