New answers tagged

4

dfhwze has addressed the structural and other issues with the code, so I will focus on eliminating the pyramid of doom in AllPasswordCodes: private static IEnumerable<byte[]> AllPasswordCodes { get { for (char a = '0'; a <= '9'; a++) { for (char b = '0'; b <= '9'; b++) { for (char c = '...


6

I'm trying to balance the code readability/size with the performance benefits, .. Your code could be optimized for readability and object-oriented design without introducing a performance penalty. The entropy generation could be written more elegantly. original code for (char a = '0'; a <= '9'; a++) { for (char b = '0'; b <= '9'; b++) { ...


1

There is a lot of code duplication between job_read and job_write, and all the work is being done from a lot of threads inside a big mutex that will only allow one thread ever to actually do something. Servicing all clients from a single thread using select(2) would be more efficient. The most annoying failure mode for this setup is that if one of the ...


2

pthread_create returns value which is needed to be checked too; if your system runs out of resources, the new thread might not start. You are listening on all IPs available; considering that there is no visible authentication, this opens possibility for unauthorized access, impersonation etc. Consider listening on localhost only (enough within one server); ...


4

regarding: if(n<0){ printf("Buffer: %s", buffer); error("Reading Failed"); } the call to printf() will change the value in errno. The result is the call to perror() in function: error() will print the wrong message. regarding: void* job_read(void * p){ int* socketp = (int*)p; int newsockfd = (*socketp); the variable ...


7

Portability The code uses pthread which is a POSIX (Portable Operating System Interface) structure. If the code uses POSIX for one thing, it might be better to use POSIX for all things to remain POSIX compliant. This means that instead of using bzero((char*)&serv_add, sizeof(struct sockaddr_in)); to zero out the serv_add variable it might be better use ...


4

Serious issues with this: Memory Management. Not using RAII to lock/unlock Memory Management Don't pass pointers it does not indicate owners. Always wrap pointers in a smart pointer. Have a look at std::unique_ptr. But for types like Data there is no need to use pointers. Simply use Data as the object type (not Data*). RAII Look up the concept of RAII. ...


3

Specification Considerations Note that the first caller that gets access is the one that provides the cancellationToken for the valueFactory. Other callers use their own cancellationToken for timing out on awaiting the result of the async operation. This is how you designed your class. I did not change this part of the behavior. If this behavior is not ...


1

Guard Arguments put does not check for e == null. Are null values allowed? If not, add an argument guard if (e == null) throw new NullPointerException();. Naming Conventions Call your class BlockingStack to conform to Java naming guidelines concerning concurrent collections. max should be called capacity. A collection's capacity is its maximum size. As ...


2

Double Check Locking is not recommended. Specially in this case where the time spend inside the lock is minimal. public bool TryCapture() { if (!IsCaptured) lock (_lock) if (!IsCaptured) return IsCaptured = true; return false; } Keep it simple.. public bool TryCapture() { lock (_lock) if (!IsCaptured)...


1

Q&A I am thinking of removing timeoutStyle and timeout from the Cache constructor and providing default values that can be changed via public properties. How do you like that? This would add additional complexity that is unlikely to be useful. Normal usage of a cache is to be a long-lived instance that is task-scheduled periodically given a fixed ...


3

Mutable default arguments sil_opts:dict = default_sil_options This can create some nasty bugs, and some Python inspectors specifically recommend against doing this. The alternative is to set the default to None, and then write an if in your function to assign the default dictionary if the argument is None. Redundant return It seems like you intend for ...


1

Consider following point: For Request.send() is thread safe as it does not used any sharing resource of Request class. As per provided code single(singleton) object will be created as per default spring configuration. You must shutdown ExecutorService once no more thread need to be add.


1

Although the motivation is to create independent unit tests, it only partially achieves that result. Because the singleton is globally accessible from while it's alive, we've constrained the tests to run sequentially, which limits our ability to run more of them in a given time. There may be benefit in making the singleton be thread-local for tests (this ...


2

While your code is thread-safe, it does not scale. Consider adding a fast-path in case there is an object. There is no need for a class, a free function is enough. std::make_shared() generally coalesces the two allocations (control-block and payload), making it significantly more efficient. If you want to ensure only get_shared_singleton() can ever construct ...


2

The problem with the above code is that it doing two things with one piece of code. 1: Singleton Creation. 2: Singelton LifeSpan. I would separate these out into two separate classes. The creation is done via an abstract factory (MySingletonFactory) but lifespan is controlled via explicit factory classes that are defined for the situation (so your code ...


1

Design The thread functions function_count1 and function_count2 are practically identical. It would be simpler to have one function that is prameterized based on some input argument. I don't like global modifiable state. Pass in the variables as parameters to your function. This allows your functions to be much more flexable in how they work. ...


1

void *function_count2(); Don't use this deprecated feature bug. It has been deprecated since C90, so I see absolutely no reason to use it, ever.


1

Testability This is my main concern when creating an API that talks to external references, in this case The CAN library. The problem with this library is that we are forced to consume a static class PCANbasic. There is no interface provided for us to work with. Testing our own code depends on a connection to the CAN bus. // static class is a tight ...


1

I think the conditional if (predicate_value != 0) in do_work() is unnecessary. The while loop only exits when predicate_value is non-zero. We shouldn't have to worry about the content of predicate_value changing between the while and the if. When pthread_cond_wait() returns it locks the mutex so predicate_value is consistent until we unlock the mutex. ...


4

System.out.println(sex + " using bathroom. Current employees in bathroom = " + size); This line is not protected by lock, so it is possible that size can change before the output is generated. For example, if two 'M' arrive simultaneously, you could get as output: M using bathroom. Current employees in bathroom = 2 M using bathroom. Current employees in ...


-25

You have implemented a bathroom that can be occupied by only one type of genital at a time. Therefore it is not a unisex bathroom but a monosex bathroom. You have labelled sex as "man" and "woman" which are genders. From what I understand the terms you are looking for here are "penis" and "vagina". The code only identifies entrants as "men" hile all others ...


4

This solution actually has a data race on count. count is accessed outside the mutex from both threads: while(count <= 10) Compile the program with the thread sanitizer (or the corresponding tool in Visual Studio) and it should throw a warning. The program doesn't necessarily exit. When I run it, thread1 hangs forever at the call to pthread_cond_wait()....


1

The problem is a combination of very large numbers and rendering in ItemsControlss in WPF. And if you think about it: No human can use the full view of a number with 10000 digits to anything. Therefore you need a view model: public class FactorialVM : INotifyPropertyChanged { public FactorialVM(long value, BigInteger factorial) { Value = ...


Top 50 recent answers are included