Search Unity

Discussion Useful and helpful error messages, please!

Discussion in 'General Discussion' started by georgeq, Nov 24, 2022.

  1. georgeq

    georgeq

    Joined:
    Mar 5, 2014
    Posts:
    662
    There was I time when computers had limited memory and programmers were force to fit their code in a very tight space, so to display an extremally compact message when an unexpected condition was found was an unavoidable necessity in most cases, moreover if that condition was rare. Messages like "Error 5" were common on that age. At the current level of hardware development and the amount of available memory and resources in general, excuses for throwing useless error messages are not longer valid. Yes, now you get something more digestible than just a code number, however it seems to me that some programmers responsible for the most basic and fundamental components are either too lazy or really don't care about their users. Take for example this message:

    Code (CSharp):
    1. ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection.
    2. Parameter name: index
    This is the perfect example of a half-useful / half-useless message, you know what caused you program to crash, but don't know why. Obviously, in order to throw this exception you must know the size of the array (or collection) and the index value that caused it, you simply cannot even detect the error without knowing these 2 values. If this message contained info about the size of the array and the offending index value, in most cases you could probably figure out right away what happened, but instead you have to run a set of tests hoping for the error to show up again under controlled conditions. The person responsible for witting the text probably thought: "not my problem, let them figure it out", which I think is a bit irresponsible.

    Consider this piece of code, which generated the referred message.

    Code (CSharp):
    1.    private List<Action> end;
    2.  
    3.    public void End() {
    4.       if(end==null) {
    5.          return;
    6.       }
    7.       for(int i=end.Count-1;i>=0;i--) {
    8.          end[i]();
    9.       }
    10.       end.Clear();
    11.    }
    At first glance it seems bullet proof, yet line 8 throws the above exception.

    I think people responsible for writing compilers and game engines should be more thoughtful when delivering error messages, you never know, probably you just catched a one in a million condition that may be quite difficult to reproduce without knowing all the relevant data. So why to make people's life miserable by throwing away a set of data that you already have and that would be quite easy to display?

    Please, no more lame excuses and make sure all your error messages are useful and helpful.
     
    Last edited: Nov 24, 2022
  2. Murgilod

    Murgilod

    Joined:
    Nov 12, 2013
    Posts:
    10,160
    No it doesn't.

    At first glance it's pretty obvious this would never work. Count-1 on an empty list will always return -1.

    edit: in fact, this is a prime example of why you should only iterate over a list negatively if absolutely necessary.
     
    Last edited: Nov 24, 2022
  3. DragonCoder

    DragonCoder

    Joined:
    Jul 3, 2015
    Posts:
    1,700
    That is similar to my question here:
    https://forum.unity.com/threads/why...ge-never-tell-which-variable-is-null.1355222/

    Guess the answer is similar too: Optimization/compilation throws away that information. In the case of index out of bounds, it can detect when the index led to a memory address that is not allowed to be accessed in the current context and it knows the code line. But it does not have all variable contexts. At least not of the array/list and its length. Though I do really wonder why it doesn't even know the value of the variable it literally just used to reach that memory address..
    To at least know whether the value was -1 or a large number would be highly valuable.

    I agree, C# could really work on that - at very least in Debug mode which already has reduced performance. Fairly sure Java does give you this information, if I recall my college years well.

    At least everything is better than what C++ gives you - ||SEGFAULT|| or
    "Compilation failed. Cause: Definition of [Insert ~300 characters of chained class names here] is missing in [Insert ~200 characters of more chained class names here].", haha. Unity is a blessing compared to my day job sometimes..

    But.. there's the check for (i >= 0) and this is a for-loop and not a do-loop which would execute without a check beforehand.

    I cannot reproduce an error in those code lines with an empty list in every case.

    EDIT:
    Have you actually executed in Debug mode?
    Because there is a "bug"/problem where the codeline is wrong sometimes in non-debug. I only encountered it exactly in index out of bounds errors. The code line given however is the end of the block in that case.
     
    Last edited: Nov 24, 2022
  4. PizzaPie

    PizzaPie

    Joined:
    Oct 11, 2015
    Posts:
    107
    This is perfectly fine and shouldn't throw any errors at all.

    As for more descriptive errors think it is a matter of keeping consistency you wouldn't want errors to blabber about stuff where in most cases this minimal info is more than enough to figure what's wrong. Then again it might not be possible or worth the effort, no clue.
     
  5. Murgilod

    Murgilod

    Joined:
    Nov 12, 2013
    Posts:
    10,160
    Yeah actually. If anything it feels like the issue is probably in the Action() being called.
     
  6. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    15,620
    Maybe, but in this case I don't know.

    The .NET environment does bounds checks on array access (which always felt redundant to me outside of debug mode) and raises exceptions. Unlike the stuff discussed in that other thread this doesn't require storing or generating any extra data, just using stuff that - as the OP says - is probably already there. It "just" needs to be passed to whatever generates the exception message.

    Of course, "just" often isn't as straightforward as it seems. Maybe there's a good reason. And personally, this message has never bothered me for the same reason the bounds checks feel redundant.

    But the OP said it's just an example. What are some others?

    Lately I'm irritated by one about domain reloads, which annoys me due to the lack of a stack trace. It's probably beveasy to fix if there was some hint as to what was causing it.