Well played sir! Done and pushed. If any of the words defined would be helpful
I'm more than happy to contribute them back. I was also thinking
parallel-product-map might be useful?
Jon
On Saturday, October 12, 2013 9:33 AM, John Benediktsson <[email protected]>
wrote:
Sometimes it is possible to "over-factor" words, you might find this simpler,
particularly because it keeps the string constants localized to where they make
sense:
: extract-symbols ( html -- seq )
> R/ q\?s=[A-Z]+">/ [
> [ "q?s=" length + ] [ "\">" length - ] [ subseq ] tri*
> ] map-matches ;
Also, I'm thinking we should make your "get-html" a word somewhere since I
always find myself doing "http-get nip" (which is lazy) and really it might be
better to have "http-get*" which checks and drops the response like you have as
a library word.
Also, you can have multiple <PRIVATE ... PRIVATE> sections, so you might find
it "cleaner" to have your (group-by-symbol) word defined immediately before it
is used in group-by-symbol.
Best,
John.
On Sat, Oct 12, 2013 at 9:03 AM, Jon Herron <[email protected]>
wrote:
I've (hopefully) made all the changes that were suggested. The code looks much
better now, thanks for all the advice. John Benediktsson had a good suggestion
to use push-at during (group-by-symbol), which made the code cleaner overall as
well. If there are any more suggestions feel free to let me know.
>
>
>
>Thanks again for all the help and advice. Going to start on the next piece of
>my puzzle.
>
>Jon
>
>
>
>
>On Thursday, October 10, 2013 8:19 AM, Jon Herron
><[email protected]> wrote:
>
>Thanks Alex. This looks like excellent feedback. I'll digest this and report
>back.
>
>
>Jon
>
>
>
>On Thursday, October 10, 2013 12:20 AM, Alex Vondrak <[email protected]>
>wrote:
>
>Overall, the code is readable. Nice work!
>
>It's a bit odd to have a separate private.factor file on its own. The
>only Factor project to do this (in my git pull from July) is
>unmaintained:
>
> $ find . -name private.factor
> ./unmaintained/alien/marshall/private/private.factor
>
>More common is to use `<PRIVATE ... PRIVATE>` syntax in the same file:
>
> $ grep -l "<PRIVATE" **/*.factor | wc -l
> 613
>
>See http://docs.factorcode.org/content/word-__lt__PRIVATE,syntax.html
>
>A few things I noticed (mind you, it's late at night for me right now):
>
>https://bitbucket.org/jherron/market-movers/src/tip/yahoo/finance/market-movers/market-movers.factor
>
>All around, you use a lot of `{ } nsequence`. For one, you can use
>http://docs.factorcode.org/content/word-1array,arrays.html For
>another, you're defining words to calculate constants. Consider
>instead using the
literals
vocab:
>http://docs.factorcode.org/content/article-literals.html
>
>https://bitbucket.org/jherron/market-movers/src/tip/yahoo/finance/market-movers/market-movers.factor?at=default#cl-38
>
>Consider using http://docs.factorcode.org/content/word-C__colon__,syntax.html
>
>https://bitbucket.org/jherron/market-movers/src/tip/yahoo/finance/market-movers/private/private.factor
>
>All around, you seem to have some long lines. The style convention is
>to keep source code 64 characters wide:
>http://docs.factorcode.org/content/word-margin,prettyprint.config.html
>
>https://bitbucket.org/jherron/market-movers/src/tip/yahoo/finance/market-movers/private/private.factor?at=default#cl-12
>
>Couldn't this be a `CONSTANT:`? I mean, if you even want to bother
>defining it as such, instead of just
inlining the regular expression
>into `extract-symbols`.
>
>https://bitbucket.org/jherron/market-movers/src/tip/yahoo/finance/market-movers/private/private.factor?at=default#cl-14
>
>Don't worry, the length is constant-folded by the compiler already:
>
> IN: scratchpad USE: compiler.tree.debugger
> IN: scratchpad [ [ "q?s=" length + ] 2dip ] optimized.
> [ "COMPLEX SHUFFLE" 4 + "COMPLEX SHUFFLE" ]
>
>That said, I think code like
>
> USE: splitting
>
> "q?s=FB\">"
>
"q?s=" ?head
drop
> "\">" ?tail drop
>
>might be clearer. But it's a minor point, and you have to balance
>that with the fact that `map-matches` is already giving you input
>suitable for `subseq`, so you might as well just use that (like you're
>doing already).
>
>https://bitbucket.org/jherron/market-movers/src/tip/yahoo/finance/market-movers/private/private.factor?at=default#cl-26
>
>product-sequences already implement the sequences protocol; I doubt
>you need to call `>array`.
>
>https://bitbucket.org/jherron/market-movers/src/tip/yahoo/finance/market-movers/private/private.factor?at=default#cl-28
>
>Consider using http://docs.factorcode.org/content/word-map-sum%2Csequences.html
>
>https://bitbucket.org/jherron/market-movers/src/tip/yahoo/finance/market-movers/private/private.factor?at=default#cl-30
>
>You could simplify the logic slightly, if you're so inclined:
>
> :
symbol-locations (
assoc key -- hash-set )
> swap [ [ HS{ } clone or ] change-at ] [ at ] 2bi ;
>
>Oh, by the way, `assoc` is a more typical stack-effect name than
>`hashtable`. See
>http://docs.factorcode.org/content/article-conventions.html
>
>https://bitbucket.org/jherron/market-movers/src/tip/yahoo/finance/market-movers/private/private.factor?at=default#cl-34
>
>`[ p ] keep q` is the same as `[ p ] [ q ] bi`. Here, `[ category>> ]
>[ group>> ] bi`.
>
>https://bitbucket.org/jherron/market-movers/src/tip/yahoo/finance/market-movers/private/private.factor?at=default#cl-37
>
>In general, I find `reduce` hard to read. It looks like you're using
>it to try to preserve & return the hashtable at the end. I'm not
>sure, and I won't hazard to rewrite the logic myself in my current
>state, but you might be able to just pass around a reference to the
>hashtable in a more conventional iteration:
>
> IN: scratchpad H{ } clone { 1 2 3 } over '[ dup 100 * _ set-at ] each .
> H{ { 100 1 } { 200 2 } { 300 3 } }
>
>Hope that
helps & happy hacking,
>--Alex Vondrak
>
>
>On Wed, Oct 9, 2013 at 8:10 PM, Jon Herron
><[email protected]> wrote:
>> That would be very cool. I'd love to contribute if anyone else would find it
>> helpful.
>>
>> The repo is @ https://bitbucket.org/jherron/market-movers
>>
>> Thanks for taking the time.
>>
>> Jon
>>
>>
>> On Wednesday, October 9, 2013 8:05 PM, John Benediktsson <[email protected]>
>> wrote:
>> Sure, send a link to your code. We'd be happy to review. Also, I have some
>> support for getting quotes from Yahoo! Finance - maybe you could contribute
>> some of your work to the project:
>>
>> https://github.com/mrjbq7/re-factor/blob/master/yahoo/finance/finance.factor
>>
>>
>>
>> On Oct 9, 2013, at 7:53 PM, Jon Herron <[email protected]>
>> wrote:
>>
>> Hi all -
>>
>> In my spare time I am working on a
backtesting application and I've
>> decided to write it in Factor. So far all I have is a vocab for
>> scraping/parsing the market movers from Yahoo Finance (eg
>> http://finance.yahoo.com/actives?e=us). Before I write too much more Factor
>> I was wondering if anyone more experienced would mind reviewing my code? I
>> feel pretty confident that the code works, I just don't know if it is "good
>> Factor". If this is an OK request I'll send the link to my repo (I don't
>> want to come across as spamming on my first post).
>>
>> Thanks,
>>
>> Jon
>>
>>
------------------------------------------------------------------------------
>> October Webinars: Code for Performance
>> Free Intel webinars can help you accelerate application performance.
>> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most
>> from
>> the latest Intel processors and coprocessors. See abstracts and register >
>> http://pubads.g.doubleclick.net/gampad/clk?id=60134071&iu=/4140/ostg.clktrk
>>
>> _______________________________________________
>> Factor-talk mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/factor-talk
>
>>
>>
>>
>>
>> ------------------------------------------------------------------------------
>> October Webinars: Code for Performance
>> Free Intel webinars can help you accelerate application performance.
>> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most
>> from
>> the latest Intel processors and coprocessors. See abstracts and register >
>> http://pubads.g.doubleclick.net/gampad/clk?id=60134071&iu=/4140/ostg.clktrk
>> _______________________________________________
>> Factor-talk mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/factor-talk
>>
>
>
>
>
>------------------------------------------------------------------------------
>October Webinars: Code for Performance
>Free Intel webinars can help you accelerate application performance.
>Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
>the latest Intel processors and coprocessors. See abstracts and register >
>http://pubads.g.doubleclick.net/gampad/clk?id=60134071&iu=/4140/ostg.clktrk
>
>_______________________________________________
>Factor-talk mailing list
>[email protected]
>https://lists.sourceforge.net/lists/listinfo/factor-talk
>
>
>
>------------------------------------------------------------------------------
>October Webinars: Code for Performance
>Free Intel webinars can help you accelerate application performance.
>Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
>the latest Intel processors and coprocessors. See abstracts and register >
>http://pubads.g.doubleclick.net/gampad/clk?id=60134071&iu=/4140/ostg.clktrk
>_______________________________________________
>Factor-talk mailing list
>[email protected]
>https://lists.sourceforge.net/lists/listinfo/factor-talk
>
>------------------------------------------------------------------------------
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
the latest Intel processors and coprocessors. See abstracts and register >
http://pubads.g.doubleclick.net/gampad/clk?id=60134071&iu=/4140/ostg.clktrk
_______________________________________________
Factor-talk mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/factor-talk
