# Tag Info

8

Here are my suggestions: import csv, redis, json import sys REDIS_HOST = 'localhost' def read_csv_data(csv_file, ik, iv): with open(csv_file, encoding='utf-8') as csvf: csv_data = csv.reader(csvf) return [(r[ik], r[iv]) for r in csv_data] def store_data(conn, data): for i in data: conn.setnx(i[0], i[1]) return data ...

6

The easiest, and what I also believe is the best as well, way to reduce cyclomatic complexity is to extract methods. Your method suffers from one particular code smell Long method: a method, function, or procedure that has grown too large. Below, I have tried to refactor your code by extracting inner sections into their own methods. The way I have done ...

6

The Singleton pattern is a way to work around the absence of global variables in Java. But Python has global variables, and it's usually possible to write shorter and simpler initialization code if you use them, for example: import os from redis import StrictRedis _connection = None def connection(): """Return the Redis connection to the URL given by ...

5

Metrics like the cyclomatic complexity are only useful when taken in the context of your code. For example, your retrieveHistory() method has numerous calls to JSONObject where it could throw a JSONException. Each of these calls adds one to the complexity (or should do). On the other hand, I think your exceptions are not overly complicated, and the code ...

5

Good use of Lazy on the connection multiplexer but consider making it a readonly field so you can't accidentally replace it later. Doing this in the Lazy's factory function is a bit odd in my opinion ThreadPool.SetMinThreads(50, 50);. I'm not sure what options you have for initialization in a Azure Function but you could do worse than use a static ...

4

The first problem with this code is that there is too little explanation for anything. There is no documentation. There is a referenced class RedisClient for which there is no explanation of what it is and how it is being used. The variable names tend to be undescriptive. When asking other people to read code, then it is the author's responsibility to make ...

4

There are some causes for concern with this code. Could you elaborate a bit more on the intended use of this, as well as what you're hoping to gain from its use? That might help clear up some of the below points. Performance It's fast in your example, but many other cases may be slower than intended. Depends on your use case which is why some elaboration ...

4

Disclaimer: I don't know squat about sparklr2, tonr2, and very little about oauth and Redis. I do have a couple of admittedly superficial remarks though. Hardcoded 1 and 2 values Why hardcode 1 and 2 to select between in-memory or cache as the store? The numbers alone are meaningless, and you could easily replace them with constants, or put both into an ...

4

I'm skeptical of accepting postal codes as Integers. Japanese postal codes are of the format NNN-NNNN. Accepting a String, which may contain an optional hyphen and leading zeros, would be more appropriate. Furthermore, an API that is designed to take an integer could never be extended to countries like the UK and Canada, where postal codes are ...

4

Simon's answer got the basic idea right and got my vote, but I think his answer left many problems intact, and may not have given enough details to actually do the method extraction. Note the following: In the retrieveHistory method jedis was not being used, which became obvious after refactoring. By getJedisObjectFromPool and returnJedisObjectToPool is ...

4

There might be one correctness issue here: If a customer makes just one purchase all week with the transaction amount exceeding the cap, do you still want to throttle the transaction? Just wanted to bring that up in case you missed this corner case. Since you talk about performance, you will want to consider which of the two are important to you: ...

4

I previously have written a helper for this, as I've come across it too. If you pass the object and the keys to the object then you can do the same. def get_nested(obj, keys) try: for key in keys: obj = obj[key] except KeyError: return None return obj This makes usage: subset['price'] = get_nested(row, 'quotes USD ...

4

I can share with you an example version of Redis service such as getting subscribers or accessing the other databases and also for catching connection issues. ConfigurationOptions allows us to initialize service with more Redis servers(Redis pool), if needed you can easily change the constructor parameters. Note that ConnectionMultiplexer.Connect(); is ...

3

if(typeof req.body.data !== 'undefined' && req.body.data ... // to if(req.body.data && req.query.hash && typeof req.body.id) Assuming that false, 0, null and an empty string has no special meaning in this program, you can simply use loose comparison. Anything falsy (null, undefined, false, 0, NaN, empty string) will just fall off. ...

3

If I focus on how ConvertToRedisHash is returning its IEnumerable<HashEntry>, I get this: public static IEnumerable<HashEntry> ConvertToRedisHash(object o, bool ignoreFail = false) { var hashList = new List<HashEntry>(); foreach (var prop in o.GetType().GetProperties()) { ... hashList.Add(new HashEntry(prop.Name,...

3

dead code like in the constructor arguments for RedisJobQueue should be deleted. To keep track of changes a version control system like GIT or SVN should be used. You are using C# 6.0 so you can make use of the string interpolation using the $operator like so public RedisJobQueue(string jobName) { _jobQueue =$"{jobName}:jobs"; _processingQueue =...

3

Disclaimer: I don't know sqat about Lua or Redis. I have just one small thing to point out. You return 0 for failure and 1 for success. Lua seems to have boolean type, so why not return false and true, respectively? I hope somebody else will give you a more insightful review!

3

One thing that you could change is the way that this if else statement is written var hostname; if(os.hostname() == "kipos"){ hostname = os.hostname()+".dev"; }else{ hostname = os.hostname(); } Here you are deciding what a variables value should be using an if/else statement, this is a good spot to use a JavaScript ternary operator. Example ...

3

I run the code in my laptop. I got 16594.76 requests per second. Environment: Ubuntu 15.10 Run Redis natively (no VM) CPU: Intel(R) Core(TM) i7-2620M CPU @ 2.70GHz Mem: 16GB After some changes I got 17825.31 requests per second. > local all = redis.call('HMGET', key, unpack(getting)) > for k, v in pairs(all) do > current_count = current_count + (...

3

As mentioned in my comment I think in general Dispose is expected to be synchronous hence it should block until finished which is easy enough to achieve: _db.LockReleaseAsync(Key, Value).Wait(); One other gripe I have with the code is the static IDatabaseAsync member of DataCacheLock. Maybe I'm missing something but I think this can get you into trouble. ...

3

The pattern match some_result { Ok(v) => v, Err(e) => panic!("message: {:?}", e), } is exactly what expect does for you. We can get a somewhat leaner main if we use that instead: fn main() { let self_posts = get_posts(20).expect("Failed to fetch new posts"); let redis_client = redis::Client::open("redis://127.0.0.1").expect(...

2

private static readonly Lazy<ConnectionMultiplexer> LazyConnection = new Lazy<ConnectionMultiplexer>(() => { return ConnectionMultiplexer.Connect("CONNECTIONSTRINGTHATWILLCOMEFROMWEBCONFIG"); }); You can simplify this statement lambda to expression lambda: private static readonly Lazy<ConnectionMultiplexer> LazyConnection = new ...

2

Definitely do not lock on the ConfigProxy instance. Somebody else could acquire a lock on it and then you'd be up the creek. Only lock on private instances that you have the only copy of. Strictly speaking, you could use putIfAbsent and get rid of the lock altogether, but then you're potentially seeing multiple Redis calls for the same key. You should check ...

2

I know quite little Scala, but even I can understand this code. That's definitely a very good sign! Postal code validation I'm wondering if the range check of postal codes in lookup is all that useful. It saves you a cache lookup, but is it really worth it? If you skip that test the method becomes shorter, and also more flexible. If someday they add one ...

2

Firstly, you've hard-coded all of your key names instead of using the KEYS table for passing them to the script - this is both hard to maintain as well as incompatible with clustering. Next, consider using MessagePack (cmsgpack in Redis' Lua) instead of JSON when serialiazing/deserializing questions (q) and users (u) - it is leaner and faster. Also, the ...

2

In AsManager you start a long running task - you should pass the TaskCreationOptions.LongRunning to StartNew to help the framework making better decisions. Also the task runs in a while (true) loop without an obvious bailout condition. It would probably a bit cleaner if you pass a CancellationToken to it so you can cancel the task and end it when shutting ...

2

Your code is alright, really. However, by your using redis, I suspect you're after a fast way to store tiny bits of data. The varname $cache somewhat confirms my suspicion. If this is the case, your using lrange might not be the best choice, nor is exists really meaningful: INCR creates a new key, if required, and it returns the new value of that key in ... 2 I see a number of things that may help you improve your code. Make sure to #include all required headers This program refers to std::vector and std::thread but does not include the corresponding headers. Fix that by adding these lines: #include <thread> #include <vector> Check return values The function rdx.connect() can fail. Your program ... 2 shellcheck it! There is this wonderful site called shellcheck.net that tells you all kinds of common mistakes. The newline in a string n=" " n is an unusual name for something that's a newline. How about newline? Here's another writing style that you might like: n=$'\n' For more details on this syntax, search for \\$' in man bash. Embedding variables ...

2

I think some simple code clean-up would go a long way here. You have a number of nested conditionals that simply don't need to exist. For example: if ( !valid_widget(widgetnum) ) { res.status(400).send("Invalid Widget"); return; } else { var keys = {}; ... There is no reason at all for your else ...

Only top voted, non community-wiki answers of a minimum length are eligible