> Looks pretty good to me. I like your use of preconditions and the  
> number of tests. I think your indentation is a little deep (I go for  
> two spaces, myself), and I never use :Capital keywords, but otherwise  
> great.
>
The indentation was just what the Clojure plugin for Eclipse was set
to by default.

As for capitalization I've never been completely clear on what the
normal naming rules in Clojure are.

> You might be interested in the `are` macro to make your test code  
> simpler:
>

Thanks for pointing this out. Very useful. I've already started
adapting some of my tests.

> One suggestion:
>
> Rather than having two phases of derived args, and calling functions  
> with arguments like:
>
>   (total-house-depreciation args-and-mib)
>
> I'd save that step and adjust total-house-depreciation, year-sold,  
> etc. to take an additional months-in-business argument:
>
> (defn total-house-depreciation
>     "The total amount of depreciation on the rental property taken  
> over the period we were in business"
>     [args months-in-business]
>     ...)
>
> Then you can change your derived-args function to:
>
> (defn derived-args
>         [{:keys [months-to-find-tenant months-in-lease lease-cycles months-to-
> sell] :as args}]
>    (assoc args
>      :months-in-business (months-in-business months-to-find-tenant  
> months-in-lease lease-cycles months-to-sell)
>      :total-house-depreciation (total-house-depreciation args months-
> in-business)
>      :year-sold (year-sold args months-in-business)
>      :months-actively-renting (months-actively-renting months-to-find-
> tenant months-in-lease lease-cycles)))
>
> No lets at all.

The reason why I didn't make months-in-business an explicit arg for
total-house-depreciation and year-sold is because both functions are
thin wrappers for underlying functions that take args. So if I broke
out months-in-business at the top I'd just have to invent the
equivalent of the args-and-mib variable farther down to give the
functions they are wrapping what they are expecting.

BTW, the only place I had to use let in anything even like this manner
was derived-args. Everywhere else I can just pass args with no voodoo,
no lets, nothing. All the 'voodoo' is just in derived-args and
creating derived-args.

> You'll see then that it's a small step from there to my suggested  
> "functional" end game, which eliminates the passing of the args map  
> altogether: derived-args would extract all the named arguments and  
> pass just the specific ones to each function. No big deal, though;  
> there are advantages to the map approach.
>

This is the one paragraph I'm not sure I completely follow. derived-
args is a pretty dumb (as in intelligence) function. It just takes the
core args (the ones passed in by the user) and augments them with some
convenience values that are static for the whole run of the
calculator.

So without the map top level functions like Rent, Sell,
MonthlyRentalProfit, etc. would literally need to take 30+ arguments
(I'm not exaggerating btw). My root function looks like:

(defn run-calculator
   [args]
   (let [derived-args (derived-args args)]
    (- (rent derived-args) (sell derived-args))))

I have to think that's preferable to submitting 30+ arguments to rent
and sell.

Or were you suggesting a different approach?

> Looking good!
>
> -R

   Mega thanks!!!

        Yaron

-- 
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clojure@googlegroups.com
Note that posts from new members are moderated - please be patient with your 
first post.
To unsubscribe from this group, send email to
clojure+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en

Reply via email to