current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote -2 down vote favorite

I have 2 connections. How can I reduce this to one connection?

//AuthentificationController class:
    public string Register(string nickName, string email, string password)
    {
       try
       {
          if(!UserWorker.IsUserRegistered(nickName)) //connect
          {
             UserWorker.RegisterUser(nickName, email, password)) //connect
             return "Done";
          }
          else
          {
             return "You are already registered";
          }
       }
       catch(Exception ex)
       {
          //log
          return "Server error";
       }
    }

//UserWorker class:
    //...
    bool IsUserRegistered(string nickName)
    {
         using(var context = new XContext)
         {
              return context.Users.Contains(x => x.NickName == nickName);
         }
    }

    //...
    void RegisterUser(string nickName, string email, string password)
    {
         using(var context = new XContext)
         {
              User newUser = new User(nickName, email, password);
              context.Users.Add(newUser);
              context.SaveChanges();
         }
    }
share|improve this question

closed as off-topic by Simon Forsberg♦, Mat's Mug♦, syb0rg, Hosch250, Phrancis Jan 30 at 17:18

This question appears to be off-topic. The users who voted to close gave this specific reason:

If this question can be reworded to fit the rules in the help center, please edit the question.
2  
Unfortunately, it's a bit hard to review code like this because the code you are showing is example code rather than code that you're actually using in a real project, which means that a lot of context has been stripped away. Unlike Stack Overflow, we prefer to look at real code instead of example code. Please see the meta question: Why is hypothetical code off-topic for Code Review? –  Simon Forsberg♦ Jan 30 at 16:44
2  
I'm voting to close this question as example code, because it has been stripped down of its context and wouldn't compile as is (var context = new XContext isn't valid C#). That said, you're using two contexts, that doesn't mean you're using two connections - you may want to read up on connection pooling and how EF is handling that. –  Mat's Mug♦ Jan 30 at 17:07

1 Answer 1

up vote 2 down vote accepted

I only see one connection. you should only have one method that registers the user. I mean that Register and RegisterUser are the same thing.

This :

public string Register(string nickName, string email, string password)
{
   try
   {
      if(!UserWorker.IsUserRegistered(nickName)) //connect
      {
         UserWorker.RegisterUser(nickName, email, password)) //connect
         return "Done";
      }
      else
      {
         return "You are already registered";
      }
   }
   catch(Exception ex)
   {
      //log
      return "Server error";
   }
}

//...
bool IsUserRegistered(string nickName)
{
     using(var context = new XContext)
     {
          return context.Users.Contains(x => x.NickName == nickName);
     }
}

//...
void RegisterUser(string nickName, string email, string password)
{
     using(var context = new XContext)
     {
          User newUser = new User(nickName, email, password);
          context.Users.Add(newUser);
          context.SaveChanges();
     }
}

should be 

public string Register(string nickName, string email, string password)
{
    try
    {
        if(!UserWorker.IsUserRegistered(nickName)) //connect
        {
            using(var context = new XContext)
            {
                User newUser = new User(nickName, email, password);
                context.Users.Add(newUser);
                context.SaveChanges();
            }
            return "Done";
        }
        else
        {
            return "You are already registered";
        }
    }
    catch(Exception ex)
    {
        //log
        return "Server error";
    }
}

//...
bool IsUserRegistered(string nickName)
{
     using(var context = new XContext)
     {
          return context.Users.Contains(x => x.NickName == nickName);
     }
}

Or better yet, you should get rid of the boolean as well and only use 1 context for the entire thing

public string Register(string nickName, string email, string password)
{
    try
    {
        using(var context = new XContext)
        {
            if (context.Users.Contains(x => x.NickName == nickName)
            {
                User newUser = new User(nickName, email, password);
                context.Users.Add(newUser);
                context.SaveChanges();
                return "Done";
            }
            else
            {
                return "You are already registered";
            }
        }
    }
    catch(Exception ex)
    {
        //log
        return "Server error";
    }
}
share|improve this answer
    
Register - class in AuthentificationController IsUserRegistered and RegisterUser - it is methods of UserWorker class –  GLeBaTi Jan 30 at 16:58
    
I am not following? can you change the methods? if you can't why are you asking us to help you change code that you can't change? –  Malachi Jan 30 at 17:20

Not the answer you're looking for? Browse other questions tagged or ask your own question.