Fork me on GitHub
#code-reviews
<
2015-06-15
>
mdallastella07:06:04

Can someone take a look to a little project I made? Is not finished yet, but I’d like to have some feedback. It’s my first almost-complete Clojure software: https://github.com/mdallastella/fen-to-graph

slipset08:06:23

mdallastella coord-list, isn't that a map on a (range 0 9)

slipset08:06:39

I've never had the ability to read loops in clojure.

mdallastella08:06:17

@slipset: coord-list should be a vector like [:a8 :b8 :c8 … :f1 :g1 :h1]

mdallastella08:06:51

probably there are better ways to do it

slipset08:06:24

Yes, I mean a map as in the function map, not the datastructure

slipset08:06:52

A thing I often look for when code-reviewing javascript is code like:

slipset08:06:35

function f(ints) {
      var result = [];
      ints.forEach(i) {
          result.push(i++);
     }
     return result;
  }

mdallastella08:06:11

instead of using map, I see

mdallastella08:06:04

let me try to rewrite it using map

slipset08:06:01

might just be that map-indexed is what you're looking for

slipset08:06:21

crap, it might actually be a reduce of some sort, since you might need to keep track of the row you're on 😞

slipset08:06:59

(map-indexed (fn [i v] (keyword (str v (mod  (inc i) 8 )))) (flatten (repeat 8 column-list)))

slipset08:06:34

Doesn't quite get what you want, but with some more math skillz it might work.

mdallastella08:06:05

(def coord-list
  (flatten
   (map (fn [n] (map #(keyword (str % n)) column-list))
        (range 8 0 -1))))

mdallastella08:06:58

@slipset what about the rest?

slipset09:06:56

This is what I was looking for:

slipset09:06:07

(map #(keyword (str %1 %2))(cycle column-list) (mapcat (partial repeat 8) (range 1 9)))

mdallastella09:06:42

@slipset: this is what I need, thanks: (map #(keyword (str %1 %2)) (cycle files) (mapcat (partial repeat 8) (range 8 0 -1)))

mdallastella09:06:59

from :a8 down to :h1

slipset09:06:41

I guess expand-row-empty-squares could get some of the same treatment

slipset09:06:56

also, I guess your number->underscore could make some use of -> or ->>

slipset09:06:51

and why (Integer/parseInt (str n)) shouldn't n suffice?

slipset10:06:36

mdallastella: kudos for using juxt!

slipset10:06:11

althugh seems like (filter board/valid-coord? ((apply juxt functions) position)) is repeated across all the pieces?

mdallastella10:06:39

@slipset: juxt is a mind blowing and amazing function 😃

mdallastella10:06:22

Yep, some code can be refactored 😃

slipset11:06:10

mdallastella: rook-moves and bishop-moves are basically the same, just a function of how they move? up-left up-right-down-left down-right vs up right down left

slipset11:06:15

Maybe the pieces could have the allowed moves as a value on them?

slipset11:06:53

or Piece could have a get-moves which would return [up right down left] for a rook?

slipset11:06:00

To me it seems a bit strange with a record called Rook and a function called rook-moves, that's sort of what I'm trying to address.

slipset11:06:54

I guess you could also make a (defmulti valid-moves type)

slipset11:06:45

and then (defmethod valid-moves fen-to-graph.pieces.King ...)

slipset11:06:17

But I'm not sure on the stylishness in mixing records/protocols and multimethods

mdallastella12:06:20

@slipset: as soon as I come home from a customer, I’ll answer you, thanks simple_smile

slipset12:06:34

no worries simple_smile

slipset12:06:00

I like reading code, and especially golfing with other peoples code simple_smile

gmorpheme21:06:37

coord-list is simplest as a for comprehension isn't it? (for [row (range 1 9) col column-list] ...)

gmorpheme21:06:22

apply concat map in fen.clj can just be mapcat