code-reviews

agile_geek 2016-12-13T08:54:36.000546Z

@roelofw I had a very quick look at code. One comment, I don't think wrapping the pmap calls in a future is necessary. It just adds complexity.

agile_geek 2016-12-13T08:57:38.000548Z

The pmap will parallelise all the calls over a thread pool and handle the semi-laziness for you, blocking as required when it's realised by the merge.

agile_geek 2016-12-13T09:01:59.000549Z

Couple of style things - it's common practice to not leave trailing close parens ) on a separate line but have them close off the expression on the same line e.g.

art-objects (-> response
                        :body
                        :levels
                        )
Would look like:
art-objects (-> response
                        :body
                        :levels)

agile_geek 2016-12-13T09:02:57.000550Z

Except slack has messed up formatting as the :body and :levels should line up under response

agile_geek 2016-12-13T09:04:00.000552Z

I might think about names of functions too. Maybe read-data-painting is now better named format-painting-data for example.

agile_geek 2016-12-13T09:04:17.000553Z

But generally you've learned a lot! Well done.

roelofw 2016-12-13T13:11:10.000555Z

Thanks for the feedback , @agile_geek

roelofw 2016-12-13T13:11:38.000556Z

I think I have to do things otherwise. On the first page I only need the image of a painting

roelofw 2016-12-13T13:12:33.000557Z

and on a second page that I still have to build. You will see the same image with a place where the details are schown.

roelofw 2016-12-13T13:13:16.000558Z

So Im thinking to delete the do-all function and make the call to the api in the home routers file

roelofw 2016-12-13T13:13:58.000559Z

Can I still use the pmap there so things are "downloaded" in parallel.

roelofw 2016-12-13T13:14:46.000560Z

When you then click on a image , then the details are pulled out of the api with the read-data-painting

roelofw 2016-12-13T13:15:18.000561Z

Because the map is not a variable , I cannot reuse it on the to build detail page

agile_geek 2016-12-13T13:16:13.000562Z

You can use pmap to parallelise calls for different urls (i.e. different painting ids) whereever

agile_geek 2016-12-13T13:18:23.000563Z

Not sure I understand the bit about the build detail page? If the idea is to render a list of paintings and then click on a link to render the detail of a specific painting, pass the key of the painting (plus any other variables) in the query sting of the href on the link. Then destructure the query params in the function that handles that route.

roelofw 2016-12-13T13:23:02.000565Z

yes. in fact for the detail page we have already desctruced it with read-data-painting

agile_geek 2016-12-13T13:24:45.000566Z

Yeah but once you've returned a response you server side data is gone. Unless you store it in a session. So if you render the link to the detail with the data you need in the query string you don't need to store session data

agile_geek 2016-12-13T13:25:45.000567Z

Remember you may have thousands (hopefully) of users at same time so one global variable storing id's of paintings might work in this site but if everyone's view was different it wouldn't

agile_geek 2016-12-13T13:26:12.000568Z

Try and make your server stateless.

roelofw 2016-12-13T13:27:03.000569Z

yep, that why I thought of first only download the numbers and the images and shown them

roelofw 2016-12-13T13:27:41.000570Z

When I user clicks on a image then there will be another call to the api which pulls out the details and shown it

agile_geek 2016-12-13T13:27:42.000571Z

What happens if someone bookmarks the details page and then goes there as the first page after a server restart?

roelofw 2016-12-13T13:28:00.000572Z

so nothing gets "stored" anywhere

roelofw 2016-12-13T13:28:32.000573Z

a nice one. I have to think about that

agile_geek 2016-12-13T13:29:45.000574Z

If you render the images with an anchor with an href that includes all the information you need (i.e. id's) then you can catch this info in the function dealing with the route, extract the id of the painting and do the detail api lookup for that painting before rendering the detail page. Nothing is stored server side.

roelofw 2016-12-13T13:30:51.000575Z

oke, in fact the same as I schould do when someone wants to see a detail page

roelofw 2016-12-13T13:33:32.000576Z

but first some more 4clojure and standford challenges

roelofw 2016-12-13T13:56:13.000577Z

and find a better layout which also works on mobile and so on. As you can see im not a front-end developer or a designer , @agile_geek

agile_geek 2016-12-13T13:56:39.000578Z

neither am i!

roelofw 2016-12-13T13:59:49.000579Z

join the club. Problems I have with most of the templates I found is that all paintings/photo's have the same dimensions. With me there are some landscape and some not. And there are not of equal width and height