Is this efficient and should I be using encapsulation in a better way?

Question

Here is the code. Please see my questions at the bottom of this post.

public partial class myClass : Other.Class
    {
        long check1parameter = CurrentSession.CurrentFile.ID;

        protected override void EnquiryLoaded(object sender, System.EventArgs e) 
        {
            disableFields();
        }
        private void disableFields() 
        {
            if (checkEverything()) {
                EnquiryForm.GetControl("Status").Enabled = true;
            }
        }

        public bool check1_method(long check1parameter) {
            bool Check1 = false;
            string stringToCheck = check1parameter.ToString();
            if (stringToCheck.Contains("something")) {
                    Check1 = true;
                }
            return Check1;
        }

        public bool checkEverything() {
            bool roleCheck = CurrentSession.CurrentUser.IsInRoles("RequiredRole");
            bool check1 = check1_method(check1parameter);
            bool checkEverything = false;
            if (roleCheck && check1) {
                checkEverything = true;
            } 
            return checkEverything;
        }
        //other methods
    }

The code is to check that someone has a role and also that a string contains a bit of info and then disable a field. I have simplified this from the actual code to outline the key points. Although the intention is only to run these simple checks and disable a field, I thought it best to create individual methods for these tasks so they can be expanded later on.

I do get an object reference error with long check1parameter being defined in that position. It was in check1_method() and worked correctly but it's something I'd like to be declared once and used across multiple areas if possible.

I also want to pass parameters\variables to check1_method rather than declaring them inside it. What's the best way to approach making check1parameter available to all methods in this partial class? It refers to another class which is linked to Other.Class in some way.

My main question is - how do I make this as efficient as possible and should I be using private in place of public anywhere here? I'm still very new to C# and haven't quite figured out encapsulation yet, so please go easy on me! :)

Answer 1

myClass doesn't need to be declared as partial unless you intend to continue implementing it in a different file.

When using a simple if statement they can be removed, for example you could write:

public partial class myClass : Other.Class
    {
        long check1parameter = CurrentSession.CurrentFile.ID;

        protected override void EnquiryLoaded(object sender, System.EventArgs e) 
        {
            disableFields();
        }
        private void disableFields() 
        {
            EnquiryForm.GetControl("Status").Enabled = checkEverything();
        }

        public bool check1_method(long check1parameter) {
            return check1parameter.ToString().Contains("something");
        }

        public bool checkEverything() {
            bool roleCheck = CurrentSession.CurrentUser.IsInRoles("RequiredRole");
            bool check1 = check1_method(check1parameter);

            return (roleCheck && check1);
        }
        //other methods
    }

In order to save yourself from declaring unnecessary bools. Beyond that you'd be sacrificing readability for fewer lines.

When it comes to public vs private, it's good practice to always specify private unless you need to access it from outside of the class. At a glance, disableFields() should probably be public, and check1_method() and checkEverything() be private.

EDIT: Also, if check1parameter is instantiated globally to myClass, then you don't need to pass it in as a parameter to check1_methods()