Skip to content

Commit

Permalink
refactor: Refactoring Design smells
Browse files Browse the repository at this point in the history
- Element and Tag: Move method/field
- QueryParser: Extract class and Replace conditional with polymorphism
  • Loading branch information
Bhishman committed Mar 18, 2024
1 parent e3dff9a commit ad9e7f5
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 30 deletions.
2 changes: 1 addition & 1 deletion src/main/java/org/jsoup/nodes/Element.java
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ public String normalName() {
* @since 1.17.2
*/
public boolean elementIs(String normalName, String namespace) {
return tag.normalName().equals(normalName) && tag.namespace().equals(namespace);
return tag.matchesTagAttributes(normalName, namespace);
}

/**
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/org/jsoup/parser/Tag.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,17 @@ public String namespace() {
return namespace;
}

/**
* Checks if the tag's normal name and namespace match the provided parameters.
*
* @param normalName The normalized name to check.
* @param namespace The namespace to check.
* @return true if the tag's normal name and namespace match the provided parameters, false otherwise.
*/
public boolean matchesTagAttributes(String normalName, String namespace) {
return this.normalName().equals(normalName) && this.namespace().equals(namespace);
}

/**
* Get a Tag by name. If not previously defined (unknown), returns a new generic tag, that can do anything.
* <p>
Expand Down
88 changes: 59 additions & 29 deletions src/main/java/org/jsoup/select/QueryParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
import org.jsoup.parser.TokenQueue;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

Expand Down Expand Up @@ -106,35 +108,18 @@ private void combinator(char combinator) {
evals.clear();

// for most combinators: change the current eval into an AND of the current eval and the new eval
switch (combinator) {
case '>':
ImmediateParentRun run = currentEval instanceof ImmediateParentRun ?
(ImmediateParentRun) currentEval : new ImmediateParentRun(currentEval);
run.add(newEval);
currentEval = run;
break;
case ' ':
currentEval = new CombiningEvaluator.And(new StructuralEvaluator.Parent(currentEval), newEval);
break;
case '+':
currentEval = new CombiningEvaluator.And(new StructuralEvaluator.ImmediatePreviousSibling(currentEval), newEval);
break;
case '~':
currentEval = new CombiningEvaluator.And(new StructuralEvaluator.PreviousSibling(currentEval), newEval);
break;
case ',':
CombiningEvaluator.Or or;
if (currentEval instanceof CombiningEvaluator.Or) {
or = (CombiningEvaluator.Or) currentEval;
} else {
or = new CombiningEvaluator.Or();
or.add(currentEval);
}
or.add(newEval);
currentEval = or;
break;
default:
throw new Selector.SelectorParseException("Unknown combinator '%s'", combinator);
Map<Character, CombinatorHandler> handlers = new HashMap<>();
handlers.put('>', new GreaterThanHandler());
handlers.put(' ', new SpaceHandler());
handlers.put('+', new PlusHandler());
handlers.put('~', new TildeHandler());
handlers.put(',', new CommaHandler());

CombinatorHandler handler = handlers.get(combinator);
if (handler != null) {
currentEval = handler.handle(currentEval, newEval);
} else {
throw new Selector.SelectorParseException("Unknown combinator '%s'", combinator);
}

if (replaceRightMost)
Expand Down Expand Up @@ -440,3 +425,48 @@ public String toString() {
return query;
}
}

interface CombinatorHandler {
Evaluator handle(Evaluator currentEval, Evaluator newEval);
}

class GreaterThanHandler implements CombinatorHandler {
public Evaluator handle(Evaluator currentEval, Evaluator newEval) {

Check notice

Code scanning / CodeQL

Missing Override annotation Note

This method overrides
CombinatorHandler.handle
; it is advisable to add an Override annotation.
ImmediateParentRun run = currentEval instanceof ImmediateParentRun ?
(ImmediateParentRun) currentEval : new ImmediateParentRun(currentEval);
run.add(newEval);
return run;
}
}

class SpaceHandler implements CombinatorHandler {
public Evaluator handle(Evaluator currentEval, Evaluator newEval) {

Check notice

Code scanning / CodeQL

Missing Override annotation Note

This method overrides
CombinatorHandler.handle
; it is advisable to add an Override annotation.
return new CombiningEvaluator.And(new StructuralEvaluator.Parent(currentEval), newEval);
}
}

class PlusHandler implements CombinatorHandler {
public Evaluator handle(Evaluator currentEval, Evaluator newEval) {

Check notice

Code scanning / CodeQL

Missing Override annotation Note

This method overrides
CombinatorHandler.handle
; it is advisable to add an Override annotation.
return new CombiningEvaluator.And(new StructuralEvaluator.ImmediatePreviousSibling(currentEval), newEval);
}
}

class TildeHandler implements CombinatorHandler {
public Evaluator handle(Evaluator currentEval, Evaluator newEval) {

Check notice

Code scanning / CodeQL

Missing Override annotation Note

This method overrides
CombinatorHandler.handle
; it is advisable to add an Override annotation.
return new CombiningEvaluator.And(new StructuralEvaluator.PreviousSibling(currentEval), newEval);
}
}

class CommaHandler implements CombinatorHandler {
public Evaluator handle(Evaluator currentEval, Evaluator newEval) {

Check notice

Code scanning / CodeQL

Missing Override annotation Note

This method overrides
CombinatorHandler.handle
; it is advisable to add an Override annotation.
CombiningEvaluator.Or or;
if (currentEval instanceof CombiningEvaluator.Or) {
or = (CombiningEvaluator.Or) currentEval;
} else {
or = new CombiningEvaluator.Or();
or.add(currentEval);
}
or.add(newEval);
return or;
}
}

0 comments on commit ad9e7f5

Please sign in to comment.