-3
\$\begingroup\$

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();
         }
    }
\$\endgroup\$
  • 2
    \$\begingroup\$ 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? \$\endgroup\$ – Simon Forsberg Jan 30 '15 at 16:44
  • 2
    \$\begingroup\$ 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. \$\endgroup\$ – Mathieu Guindon Jan 30 '15 at 17:07
2
\$\begingroup\$

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";
    }
}
\$\endgroup\$
  • \$\begingroup\$ Register - class in AuthentificationController IsUserRegistered and RegisterUser - it is methods of UserWorker class \$\endgroup\$ – GLeBaTi Jan 30 '15 at 16:58
  • \$\begingroup\$ 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? \$\endgroup\$ – Malachi Jan 30 '15 at 17:20

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