What do you gouys think about this code?

elighne

New member
Joined
Nov 11, 2019
Messages
1
Programming Experience
Beginner
Hello!, I'm doing some experimental code and personal self-taught study.
This block I'll show you it's part of a 'game' for Unity.

So, I have a more a request than a question: I would like some criticism on this code. Anything that you can think about the usefulness, pro-tips, curiosity tips, alternatives, syntax... anything would be great.

The reason is that I have always so many questions on so many perspectives, that i can never adress them all on a single question and I end up solving it by myself, which kind of feels like re-inventing the wheel. Also I have not much time to be waiting and checking for answers (and i solve it in "less time" but more effort and therefore, less time actually experimenting and learning more). So doing this i feel like it could improve many other things based on what I already know and saving a lot of time on explaining my actual understanding to be explained what i need to understand...

I hope i made myself clear with that explanation because I would like to clarify a couple things:

1- Anything you tell me, will do. Maybe more, maybe less yeah. But for the reason mentioned above, I won't answer this post too much. However:
2- I will gladly answer any questions you may want to ask me, as soon as i read them, but I will only answer to help you answer me. Therefore, if your question is, for example, about why do I do anything or so, I wont answer., because that doesn't help you from answering me, just excuses you from not answering and I need code critics, not life choice critics or ways of learning, etc. Thanks in advance for the understanding about this.

3- Sorry for the bad english. Not my native language.

C#:
public MouseButton DragButton
        {
            get
            {
                if (dragButton == MouseButton.None)
                {
                    dragButton = MouseButton.Left;
                }
                return dragButton;
            }
            set
            {
                try
                {
                    if (value == MouseButton.None)
                    {
                        string msg = string.Format("DragButton Can't be None. It will be set to its default value: ({0}).", dragButton);
                        throw new UnauthorizedAccessException(msg);
                    }
                }
                catch (UnauthorizedAccessException) { value = dragButton; }
                finally { dragButton = value; }              
            }
        }
 
Last edited:

JuggaloBrotha

Staff member
Joined
Apr 23, 2011
Messages
168
Location
Lansing, MI; USA
Programming Experience
10+
Right off the bat I'm seeing 3 areas:
1. In your post I don't see anything about what you're expecting your code to do, so if we're to look at it we have no idea what the intention is to be.
2. In your code I don't see any code comments, granted this is a short snippet that's easy to figure out what it's supposed to be doing that doesn't excuse the lack of comments in the first place. If we're to understand your code we need to see comments that at least clarify/explains what it's supposed to be doing.

3. Seeing this in your post is concerning:
2- I will gladly answer any questions you may want to ask me, as soon as i read them, but I will only answer to help you answer me. Therefore, if your question is, for example, about why do I do anything or so, I wont answer., because that doesn't help you from answering me, just excuses you from not answering and I need code critics, not life choice critics or ways of learning, etc. Thanks in advance for the understanding about this.
Because of the lack of my points #1 & #2 inevitably the first thing anyone is going to to is ask what your intent is and, as quoted, you're saying you wont answer those kinds of questions; which leads me to believe you're expectations are widely different than what's most likely to happen which is: virtually no one is going to look at this nor is anyone likely to help you.
 

Sheepings

Senior Programmer
Joined
Sep 5, 2018
Messages
655
Location
UK
Programming Experience
10+
First, welcome to the forums.

You are coming to a self-help community learning forum and asking for opinions on code you claim you've written. You also ask for critique from the community on your code without us understanding your codes actual purpose or its intended functionality, and all because you've never explained it to us. If you're not bothered to explain it, then I am not bothered to ask you about it or give you feedback on the code you posted. I've worked with Unity 3D for years, and I would be happy to help you if you would be more forthcoming and lose the snottiness in your next post. For your first post on a new forum board, you are coming across a little rude.

There are many people here who can give you good critique or advice but in the same breath you say that you will not answer certain questions if asked by anyone who replies to you with another question. So just what is the purpose of your topic? Maybe you've missed the ball on this one, but the way it works around here, is; you post your code, and explain what it's meant to do. You then explain what your expectations are, and then explain the problem you're experiencing.

You remind me of one of those spoiled little children who has just been banned from another forum and now you've come here and brought some extra baggage with you. So If I can give you some friendly advice; ditch the attitude in which you wrote this topic and try to be a little more friendly and community orientated, so that we can actually interact with you and possibly help you, should you need it. Should you choose not to reply, then good luck to you.
 
Last edited:

Skydiver

Well-known member
Joined
Apr 6, 2019
Messages
646
Location
Virginia Beach, VA
Programming Experience
10+
This is just dumb:
C#:
                try
                {
                    if (value == MouseButton.None)
                    {
                        string msg = string.Format("DragButton Can't be None. It will be set to its default value: ({0}).", dragButton);
                        throw new UnauthorizedAccessException(msg);
                    }
                }
                catch (UnauthorizedAccessException) { value = dragButton; }
                finally { dragButton = value; }
Because:
1. You throw an exception and then immediately catch it.
2. Exceptions should not be used for flow control.
3. You are throwing the wrong kind of exception. You try one of the descendants of the ArgumentException: ArgumentOutOfRangeException, or InvalidEnumArgumentException, perhaps?
4. You didn't show us how you declared and initialized dragButton. If you just declared it and didn't initialize it, then it would default to MouseButton.None. So now look at the sequence of events: Caller trying to set the DragButton = MouseButton.None;, and then you try to create an exception saying that it can't be None, but you will set it it's default value of None.

And if you say in response to #4 that you declare and initialize dragButton = MouseButton.Left;, then your getter is pointless, since the user can never set dragButton to None. If you are paranoid, throw in an Debug.Assert() in there, but you shouldn't need that if statement in your getter.
 
Top Bottom