Re-factor parsing of the Linearization dictionary #5023
Conversation
| } | ||
| // Shadow the properties. | ||
| shadow(this, 'length', length); |
There was a problem hiding this comment.
use this.length = length; without shadowing (here and below)
|
Looks good with comments addressed. |
|
Comments have been addressed. /botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/83f0e69d4ce851e/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/5f017c73073d948/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/83f0e69d4ce851e/output.txt Total script time: 21.15 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/5f017c73073d948/output.txt Total script time: 23.19 mins
|
| this.mainXRefEntriesOffset = this.getInt('T'); | ||
|
|
||
| // Optional parameter, default value = 0. | ||
| this.pageFirst = (this.linDict.has('P') ? this.getInt('P', true) : 0); |
There was a problem hiding this comment.
nit: two comments above about setting properties and optional parameter are obvious we can remove those (also extra line breaks between properties assignment can be removed as well)
|
The second round of comments have been addressed, the patch just keep getting smaller ;-) |
|
Hmm, just found out that this patch raises an exception when linearization is not present. We need to restructure to avoid |
|
That means Linearization will be just a data class without logic, which allows not to store stream or parser reference in it. |
|
With the current "class-like" implementation of var Linearization = {
create: function (...) {
...
if (nonExistent) {
return null;
}
return {
// Properties
}
},
getInt: function (...) {
...
},
getHints: function (...) {
...
}
};At core.js#L365, we would then instead just do |
Yep. You may also make getInt and getHints as regular function. It is up to you to keep Linearization as pure static utility class or have class with only data fields. In both cases you will have create() method. |
|
@yurydelendik I've pushed an updated version that attempts to address your comments; please check if this is what you had in mind! |
|
Yep. It looks good. /botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 0 Live output at: http://107.22.172.223:8877/4a7cd41270d717d/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 0 Live output at: http://107.21.233.14:8877/72dd4f6d39f422c/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/4a7cd41270d717d/output.txt Total script time: 21.12 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/72dd4f6d39f422c/output.txt Total script time: 22.88 mins
|
|
/botio-windows preview |
From: Bot.io (Windows)ReceivedCommand cmd_preview from @yurydelendik received. Current queue size: 0 Live output at: http://107.22.172.223:8877/58fd885413430d0/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/58fd885413430d0/output.txt Total script time: 1.03 mins Published
|
Re-factor parsing of the Linearization dictionary
|
Thank you for the patch |
This PR attempts to implement what @yurydelendik suggested previously in #4145 (comment).
Fixes #4143.