You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pdfbox.apache.org by "Vivek504 (via GitHub)" <gi...@apache.org> on 2023/11/18 21:56:33 UTC
[PR] move method refactoring. [pdfbox]
Vivek504 opened a new pull request, #171:
URL: https://github.com/apache/pdfbox/pull/171
### Move Method Refactoring
**Reason:** Feature Envy
1. readSimpleValue is more interested in members of the type: Type1Font
2. readFontInfo is more interested in members of the type: Type1Font
3. readPrivate is more interested in members of the type: Type1Font
4. readProc is more interested in members of the type: Type1Lexer
5. readProcVoid is more interested in members of the type: Type1Lexer
6. readMaybe is more interested in members of the type: Type1Lexer
**Logic of Refactoring:**
1. For methods (readSimpleValue, readFontInfo and readPrivate), the code which is only dependent on Type1Font, that is moved to Type1Font class by creating a new method (readFontAttributes).
2. readProc, readProcVoid and readMaybe: these methods are removed from Type1Parser class and moved to Type1Lexer class as all 3 methods are only interested on the members of Type1Lexer class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] move method refactoring. [pdfbox]
Posted by "THausherr (via GitHub)" <gi...@apache.org>.
THausherr commented on PR #171:
URL: https://github.com/apache/pdfbox/pull/171#issuecomment-1817759724
This introduces a new public method in a class :-(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] move method refactoring. [pdfbox]
Posted by "THausherr (via GitHub)" <gi...@apache.org>.
THausherr commented on PR #171:
URL: https://github.com/apache/pdfbox/pull/171#issuecomment-1818493168
Sorry if I'm wrong, but I'm wondering whether this is a kind of public test of a new product on open source repositories to see how the humans involved react.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: [PR] move method refactoring. [pdfbox]
Posted by "lehmi (via GitHub)" <gi...@apache.org>.
lehmi commented on PR #171:
URL: https://github.com/apache/pdfbox/pull/171#issuecomment-1818364479
The parser is based in the lexer/parser principle. The lexer knows the lexical structure of the format and the parser itself uses the lexer to read an interpret the content of the files using the format. The Type1Font is the result of the parsing and doesn't need to know anything about the file format itself. According to that principle the moved methods end up in the wrong class. If we really want to change something, we might merge the lexer and the parser class, but in the moment I don't see any real reason to do so.
The implementation of readFontAttributes is, no offense, a no go. It is a wrapper for accessing the member values of the class. The huge switch statement would trigger our sonar scanner. A better approach is to introduce a setter for each value or an ever better solution is to introduce an additional class wrapping a couple of values which belong together, e.g. FontDirectory and FontInfo. Those classes could be used to initialize the Type1Font.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org