31

This might be against the spirit of Code Review, but you don't need to write your own single instance manager for WPF! Microsoft has already written code to accomplish this, but it has been poorly advertised. Microsoft's single instance manager is extremely comprehensive, and I have yet to find any issues with it. (And if you don't want to use it, it can at ...


25

No, it is simpler to code than that: enum Singleton { INSTANCE; // instance vars, constructor private final Connection connection; Singleton() { // Initialize the connection connection = DB.getConnection(); } // Static getter public static Singleton getInstance() { return INSTANCE; } ...


25

You could simplify the above significantly: class FGSingleton { static let sharedInstance = FGSingleton() var gameScore: Int = 0 // METHODS private init() { println(__FUNCTION__) } func displayGameScore() { println("\(__FUNCTION__) \(self.gameScore)") } func incrementGameScore(scoreInc: Int) { self....


20

You shouldn't be sharing Connection instances, because ADO is already doing the job for you, it uses object pooling behind the scenes. Sharing the Connection object will make it very hard to clean up your resources after you are done with it. Use interfaces instead of concrete implementation.Use IDbConnection instead of MySQLConnection You should use ...


18

First and foremost, printing text to the console is absolutely pointless for an iOS application. It's okay to do it as an easy way to test whether our code is working as intended, but we don't want to mistakenly leave this in the final release build. So, step 1: Follow the instructions in this StackOverflow post to set up a custom compiler flag for your ...


11

General rules of thumb: In order to take advantage of connection pooling (and you should), database connections should be as short lived as possible. Create, use, then immediately destroy. Single instance objects should always be agile (defined as not holding system resources e.g. db connections, file handles, etc.). If you need a single instance object to ...


11

Just commenting on ... the comments in the source (sorry): I think they add a lot of noise, and don't provide any improved clarity (in this case). They're mostly restating what the following line of code obviously does, or repeating what a variable name already describes. Such comments "by habbit" are actually harmful, as they have no benefit and could ...


10

Your code doesn’t compile, and if it would compile, it would crash. You are trying to assign a shared_ptr to a raw pointer. You are also trying to delete a shared_ptr and this leads me to believe that you don’t understand what a smart pointer actually does. In fact, your code contains a multitude of errors. More on that below. First, about the use of ...


9

This is the the way singletons are usually implemented in apple's code samples. + (ItemsManager *)sharedInstance { static ItemsManager *sharedInstance = nil; static dispatch_once_t once; /* Doing the allocation inside dispatch_once queue ensures the code will run only once for the lifetime of the app. */ dispatch_once(&once, ^{ ...


9

Okay so first of the obligatory Singletons are bad practice so you probably shouldn't make it easy to write bad code. Ignoring the fact that the class probably shouldn't exist at all we can look at the code. static T& GetInstance() { static MemGuard g; // clean up on program end if (!m_instance) { m_instance = new T(); } return *...


9

Singletons make it hard to test your code, and in my job I'd reject this at review for encouraging the development of untestable features. That said, I'll continue reviewing despite that. No need for helper class The MemGuard appears to be a poor man's reimplementation of std::unique_ptr. It would be much simpler for you to declare m_instance as a std::...


8

The best pattern for singleton that is guaranteed to be thread safe and minimizes lock overhead (because it has none) is this: public class Singleton { private Singleton() {} private static class InstanceHolder { static Singleton INSTANCE = new Singleton(); } public static Singleton getInstance() { return InstanceHolder....


8

You're misunderstanding how the static keyword works. The first time it is encountered for a specific variable in the method it defines the $inst variable as null, then checks to see if it is null (it is :p) and then sets it to a new object of that class. The second time, the static definition isn't used because the static variable has already been defined....


8

Let me start by saying I have very limited Android experience. That said, I did look into the SoundPool class a bit and have some questions/comments about your implementation: If you aren't already aware of some of the reasons why people consider Singletons an anti-pattern, I suggest you do some research on the topic and make sure you're ok with the ...


8

I totally agree with Ocelot20, but wanted to point out some more things and provide some comments of my own about some things that has been already mentioned: About mAvailableSounds: Vector is deprecated, use List (interface) and ArrayList (implementation) instead. private List<Integer> mAvailibleSounds = new ArrayList<Integer>(); Does the ...


8

ConcurrentMap As was pointed out in the comments: in the Multiton.get() method: public <V> V get(final K key, Class<V> type) { // Has it run yet? Object o = multitons.get(key); // A null value means not yet. if (o == null) { // Use a lambda to only do the create if it is still absent. o = multitons....


7

The simplest Singleton since Java 5.0 (2004) is to use an enum. public enum SomeSingletonClass { INSTANCE; } Classes are loaded lazily and if the only purpose of the class is to be a singleton, you don't need additional lazy loading. However in your cause you only need a Utility class. enum Log {; public static void info(String text) { } } Log....


7

This: Or should I just don't care about it and allow user to create instance of class? If you plan to write unit tests, which is recommended these days: for your class AND the code that will use it. http://googletesting.blogspot.co.uk/2008/05/tott-using-dependancy-injection-to.html


7

You can use a static weak_ptr in the function to cache the shared instance without extending its lifetime: template<typename T> std::shared_ptr<T> get_shared_instance() { static std::mutex m; static std::weak_ptr<T> cache; std::lock_guard<std::mutex> lg(m); std::shared_ptr<T> shared = cache.lock(); if (...


7

Singletons are evil Even if you do want to use it, use Lazy<T> instead of manually doing double-checked locking Best solution would be to leverage IoC container to inject dependencies you need.


7

Do I have too many class methods? Do I have class methods that should/could be instance methods? There's not a specific number of class methods you should have. Don't try counting your class methods or your instance methods. Class methods do class level things and instance methods do instance level things. One thing that's for certain, a class that has ...


7

Singleton I wouldn't call your Database class a singleton wrapper, as you can have multiple instances of it (not of the PDOObject field, but of the Database class). See here and here for actual singleton pattern. I don't think that your approach is necessarily bad though. Is this overkill? I think that you are a bit dishonest in your example. Your not ...


7

First, you can use implicit typing when the type is easily determined from the RHS, so: DataTable _resultTable = new DataTable(); becomes: var _resultTable = new DataTable(); etc. This saves you changing the type of a variable in more than one place (declaration and instantiation). Secondly, I'd recommend some abstracting, particularly the MySQL stuff. ...


7

General Notes I had to look at your code quite hard before I figured out what it does. Despite having variable names and method names that are descriptive, I still found it hard to 'get it'. Comments describing that the messages array is essentially a ring-buffer, and that the Counter is only used in the Stream, etc. Threading and concurrency - don't mix ...


7

As my comment explains, there are some things about your code I definitely don't understand. This answer will focus on the parts that I know need to be changed. So, starting from the top... #define kCKGeneralizedItemsDefaultTheadName "com.yourAppHere.Bundle" #define kCKGeneralizedItemsDefaultArchivingName @"yourAppKeyedArchiving" #define ...


6

I believe that some generalizations are to be made lightly. i.e. stating that Singletons are evil in general is not accurate, as there are some cases where Singleton is a proper (and maybe the only) solution. And yes I do believe that it is one of the overly abused patterns out there. Having gotten that out of the way, I move to answer your question. From ...


6

The notifyListeners() method is private, and is never called anywhere. It should be synchronized on the same lock than the other methods using the list of listeners. Unless lazy instantiation is really needed, I wouldn't use it. Just use the following code: private static final ObserverSingleton INSTANCE = new ObserverSingleton(); public static ...


6

public enum Singleton{ INSTANCE; } This is enough; you can directly use Singleton.INSTANCE to get the instance of the class.


6

SingleRandom is a really bad name for your class. Single means float in C# context, so your name implies that its a Random which generates float values, which is not true. ThreadSafeRandom or SynchronizedRandom are the examples of better naming. Are you sure that using SpinLock in you case improves performance in any way? Somehow i think that a simple lock ...


6

You're asking a rather specific coding question, and not actually for your code to be reviewed. However, I'm going to, because if you want to learn OOP, you might aswell learn what not to do. Your basic question is answered in the static-tag wiki which, cough I wrote cough. Be that as it may, your singleton implementation still has a few omissions: You can'...


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