New answers tagged

3

Welcome to Code Review, some considerations that can help you to simplify your code: you have an alphabet of 26 letters, so if the shifting operation is limited to a 26 characters range you can use the mod operation %26 to determine the final position. you can have a shift negative operation num (ex. -5), for a 26 set of elements shift this is equal to a ...


0

Probably late, but here's a re-write of seqn. (defn sequence-parser "Returns a parser that runs given parsers in order, and returns result in a sequence." [& parsers] (fn [stream] (reduce (fn [[acc stream] parser] (let [[x rest-stream] (parser stream)] [(conj acc x) rest-stream])) [[] stream] ...


2

I like that you are trying to not use explicit recursions everywhere. That being said, there are a few standard functions that we could use to get a code similar to the following. import Data.List(intercalate) grid :: Int -> [String] grid n = [intercalate " --- " . take n . drop (n-k) $ osxs | k <- [1..n]] where osxs = replicate n "O" ++ ...


3

Instead of transforming everything to String using Show just for the purpose of hashing it, you should constraint element types to be Hashable instead: getHashes :: Hashable a => BloomFilter -> a -> [Int] getHashes bloomFilter elem = let seed = hashSeed bloomFilter maxSize = m bloomFilter in (`mod` maxSize) . abs . (`hashWithSalt` ...


-1

After taking the course Functional-Light JavaScript v3, I have attempted to write a declarative/functional FizzBuzz implementation too. In this course, Kyle Simpson recommends gradually converting to declarative style, which I found helpful. Also it resulted in a way over-engineered implementation, but I think it does show parts of the code that I really ...


2

Linting You should run some sort of linter or static analyzer on your code. Rubocop is a popular one, but there are others. Rubocop was able to detect almost all of the style violations I am going to point out (and even some more), and was able to autocorrect almost all of them. Testing There is no automated testing in your code. Apart from the single ...


0

I don't think you are doing functional programming in a good way here. If you have to jump through too many hoops to inject state into your functions, and have functions that have side-effects and explicitly return None, then this is probably not functional programming. The easiest solution would probably be to write a Repo class, that consolidates all ...


1

const algo = (arr, idx) => { while (idx < arr.length - 1) { if (arr[idx] === 0) return false; idx += arr[idx]; } return true; } shorter and cleaner, edge cases skipped


5

Rather than using *args you can supply a default positional only argument. def temporary_cache(fn=None, *, ttl=60): ... if fn is not None: return decorator(fn) return decorator If you feel following "flat is better than nested" is best, we can use functools.partial to remove the need to define decorator. def temporary_cache(fn=None, *, ...


1

This uses collect since reduce is meant for collecting with an immutable result and this is validated to use with parallel streams. return schedules() .map(s->ProgramEnrollment.from(s)) // same as @RobAu .sorted(Comparator.comparing(ProgramEnrollment::getStartDate)) .collect( ArrayList::new, (c, e)->{ if (c.isEmpty()) { ...


1

I think this question may be off-topic, as it does not consist of complete working code to be reviewed. I spent (too long) trying to get it to compile, and got as far as this: https://godbolt.org/z/qCE9S8 But you have a lot of problems with the current design. Most notably, you're using raw new all over the place, which creates pointers to the heap; but you ...


2

This is a nice candidate for group-by/map-reduce! (we need an additional stream with map to get rid of the optional that is standard in reducing) Idea The idea is to group each StudentSchedule by program, map them to a small ProgramEnrollment, then to reduce the values of each key by merge or coalesce these ProgramEnrollments and finally return a list of ...


2

I only have one minor nitpick: the definition of getMaxSize becomes clearer if we use logBase: abs $ ceiling $ fromIntegral n * (log p) / (log (1 / (log 2 ^ 2))) becomes abs $ ceiling $ fromIntegral n * (- 0.5) * logBase (log 2) p We can use the identity ceiling (-x) == - floor(x) to get abs . floor $ fromIntegral n * 0.5 * logBase (log 2) p


Top 50 recent answers are included