• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 420
  • Last Modified:

What's the problem of these codes or better way to write it

hi All,

here is the code review question i got it

1.      What is wrong with this statement?

       public static ulong SecondsInADay = 86399;

2.      What is wrong with the first line of this method (I’ve changed some of the names etc., so it’s not bad naming convention)?

       public static Response DoStuff(ADUser user)
                string search = "(cn=" + user.name.Trim() + ")";


3.      What would be a better way of doing this?

        public static string GetDomain()
            return "DLA.tcs.net";

Thanks in Advance
2 Solutions
AndyAinscowFreelance programmer / ConsultantCommented:
>>here is the code review question i got it

Is this homework?  (We can't answer for you, we could help you to correct things you say howver).
Sha1395Author Commented:
AndyAinscow, its not a home work anyway, just try to figure out the expert answers.
AndyAinscowFreelance programmer / ConsultantCommented:
The first statement is just incorrect.  (Either the name is misleading or it is correct but the value given is wrong).  (Also have variables as public is not recommended - use get/set - and it is probably is meant to be a const).

The second and third use hard coded values (BUT that might not be a problem).

In general only use static when it is really required.  (If you don't understand what static does then look in the help files, it has a rather special effect.)
Cloud Class® Course: MCSA MCSE Windows Server 2012

This course teaches how to install and configure Windows Server 2012 R2.  It is the first step on your path to becoming a Microsoft Certified Solutions Expert (MCSE).

Your first code is not CLS compliant.

In the second case, you are risking an NullReferenceException, as user.name might be null, and the call to Tream() will fail.

I don't understand the las one.. But to get current domain, the right way is:

Best regards.
käµfm³d 👽Commented:
In general only use static when it is really required.
I have come to learn that there are varying schools of thought on the usage of static. One of those schools says to use static whenever there is no reason to maintain state (something classes excel at doing). In that case, the method would satisfy that requirement assuming the "..." doesn't maintain any sort of state.

What would be a better way of doing this?
I might do a readonly property:


public string Domain
    get { return "DLA.tcs.net"; }

Open in new window

AndyAinscowFreelance programmer / ConsultantCommented:
Don't forget if the name in the first code segment is correct then number of seconds in a day is 24x60x60 which is NOT 86399
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

Join & Write a Comment

Featured Post

Cloud Class® Course: C++ 11 Fundamentals

This course will introduce you to C++ 11 and teach you about syntax fundamentals.

Tackle projects and never again get stuck behind a technical roadblock.
Join Now