32

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 ...


19

Your implementation does the trick. For what it's worth, I would consider your implementation the current "traditional way". It is thread-safe. The static constructor is guaranteed to run only once, so you won't accidentally end up with two instances if two threads try to grab the instance for the first time simultaneously. Laziness There is one more thing ...


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 ...


11

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 *...


11

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::...


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

There's a potential bug with this scenario. According to this Jon Skeet post, if an exception is thrown inside the static constructor, it is never retried. Which means that if your Singleton initialization has a problem, your Singleton is doomed for the lifetime of your application, which normally isn't a problem with the "traditional" way of doing it.


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

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

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

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

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'...


6

There's very little code here to review, and I made some points in my comment to the question regarding this, however, on what is posted, here are some notes... First of all, Xcode now autosynthesizes, so you can eliminate this line: @synthesize firebaseURL, currentUserId, presenceManager, chatManager, channelManager; This does mean you'll have to refer ...


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