A few issues with your code:
WriteLine is incorrect unless you've created that method
I think you're looking for Console.WriteLine(...) which you can
use after adding using System; however even that would be
incorrect.
I assume you are looking to print the values of fields in
the User class which in that case, either override .ToString
(bad idea) or access and print them separately.
RegisterUser has no return type
It could be void -> public static void RegisterUser(string username, string pass, string add, int contact) indicating that nothing is returned.
However, common convention and expectation would be that the new User object is returned so that the caller can know what the final state of the operation was ->
- You have a rogue
; at the end of the implementation of RegisterUser(...)
You need to remove it to make your code compile.
- You actually have no variables anywhere, which would allow you to create a collection
You need to add your users to something like a List<User>, which would be encapsulated internally in another class perhaps called UserManager. This allows you to expose behaviour but hide the implementation, as well as adhering to SRP.
You could then expose the collection of users if you needed to, in a clear way, using a ReadOnlyCollection<User>.
This would show consumers that they would probably have to call a method (RegisterUser) to be able to add to the collection of users as ReadOnlyCollection<User> prevents modification, and not just do users.Add(...) & bypass your registration logic.
In this case, RegisterUser would also not have the static keyword, as it would need access to the instance field - your collection of users - and it won't be able to do that as a static method.
- Your namespace is extremely specific to your
User class
It's technically okay but namespaces are used to organise classes & control scope. I would rename it to something more related to your domain, as opposed to something bound to your class name (UserClass).
- Arguments for
RegisterUser
I would also cut down on the number of arguments to RegisterUser, take in a User object and then enforce all fields being set using the constructor for User.
This would turn it into a monadic method, making the code more readable, easier to test later on and makes you keep a conscious tab on how many "things" the method is responsible for.
Something like the below should work:
using System.Collections.Generic;
using System.Collections.ObjectModel;
namespace MyApplication
{
public class User
{
public string UserName;
public string Password;
public string Address;
public int ContactNumber;
public User(string username, string pass, string add, int contact)
{
UserName = username;
Password = pass;
Address = add;
ContactNumber = contact;
}
}
public class UserManager
{
private readonly List<User> _users = new List<User>();
public ReadOnlyCollection<User> GetUsers()
{
return _users.AsReadOnly();
}
public User RegisterUser(User newUser)
{
// process user, modify fields, add etc.
_users.Add(newUser);
return newUser;
}
}
public static class UserOutput
{
public static void WriteUserToConsole(User user)
{
Console.WriteLine($"{user.UserName}, {user.Password}, {user.Address}, {user.ContactNumber}");
}
}
}
var userManager = new UserManager();
var userToBeRegistered = new User("A", "B", "C", 0);
var createdUser = userManager.RegisterUser(userToBeRegistered);
UserOutput.WriteUserToConsole(createdUser);
var allUsers = userManager.GetUsers();
foreach (var user in allUsers)
UserOutput.WriteUserToConsole(user);