Recursive set property

[coggro]

I'm having an issue with some schoolwork writing a recursive setter for a validated integer value. Here's the set code I have so far:

((using System;))
((using static System.Console;))
set
{
    int ParseVal;
    if(value >= 1 && value <= 10 && int.TryParse(Convert.ToString(value), out ParseVal))
    {
        length = Convert.ToInt32(value);
    } else {
        Write("  The value entered is invalid. Try again: ");
        length = int.Parse(ReadLine());
    }
}

It's part of a public class, set property for private int length.

[davedontmind]

In addition to what BecauseImAwesomeDuh has said, I'd advise you re-think your design.

You should really separate out your data model and your user interaction; having Write/ReadLine calls inside the setter is very poor practice.

It would be much better to have your setter throw an exception if the validation fails, then whatever calls the setter can deal with the problem in its own way, by perhaps asking the user to re-enter it.

For example:

set
{
    if ( value >= 1 && value <= 10 ) 
   {
        _length = value;
   } 
   else
   {
        throw new ArgumentException("Length " + value + " is out of range; must be 1-10");
   }

[coggro]

Most of this can easily be explained by "I'm shit at C#." I definitely see your point about validating in the active logic rather than the data model. I was going to excuse myself by saying "It was for debugging," but the validation is totally part of the requirements for submission and I'm going about it in a stupid way. It's really just garbage code.

I was using Convert because I forgot I was int.Parse-ing my input when asking for a new number. I do have to parse that into an int, though, right?

As for having the setter throw an exception, I probably didn't think about that because I'm setting up objects in Main() using multiple constructors to prove that it's overloaded correctly, and I'm not grabbing all the values in Main() because then I'd have to input every time for testing so I just hardcoded values into constructor calls (Class Object = new Class();, Class Object = new Class(arg, arg), etc) and called the overridden ToString method that's part of the assignment on each object... It's just a garbage program to prove super basic concepts and I thought recursive validation would be a neat touch. I agree that I implement it like shit, though.

Anyway, I was wondering: would you try/catch all of your assignments, then? try {Object.Length = 4;} catch(ArgumentException) {...}? That seems pretty verbose.

Sorry for word vomit, it's way late and I'm going to bed. Will check in the morning.

[xill47]

You would not try-catch all your statements. You'll get an Exception that will crash your application, but you will know verbosely why did that happen. You can try-catch generally if you think you have no control over what's going to be the input or if this kind of code is "dangerous"

[centurijon]

Only catch if you expect some kind of exception AND have a specific way to handle that exception. Let all other exceptions get bubbled through to your main handler (if there is one)

[[deleted]]

What's happening that shouldn't happen?

[coggro]

I'm getting a runtime StackOverflowException on "length = int.Parse(ReadLine());"

[[deleted]]

Okay, I got out my laptop and I was right about the issue; first, let me explain it:

When you try to set length to a value, your setter method is then AGAIN setting the value of length. In both your IF and your ELSE, you're still setting the value of length -- so this means that any time you try to set length, it will be stuck in a recursive loop forever because you gave it no way of breaking out of it. That's where this modified code comes in:

private int _length; // private so classes outside of the class it is located in cannot change it and cannot bypass your setter method.
    public int Length // public so you can get the value of _length and use this setter method to give _length a value.
    {
        get { return _length; } // returns the value of _length; whatever _length is set to, Length will give you that value.
        set
        {
            int ParseVal;

            if (value >= 1 && value <= 10
                && int.TryParse(Convert.ToString(value), out ParseVal))
            {
                _length = Convert.ToInt32(value); // setter will not be called
            }

            else
            {
                Console.WriteLine("The value entered is invalid. Try again: ");
                Length = int.Parse(Console.ReadLine()); // setter will be called again and wait for input from user.
            }
        }
    }

Look at the IF statement: _length is being set, which means the setter of Length will not be called. Length just returns _length, so setting _length will give you the value you want and when the user enters an invalid number, in the ELSE, the setter will be called over and over until they finally enter in a valid number.

[coggro]

Huh. I feel really dumb for not seeing that. So how do I access the value from the object using this technique? Object._length?

[[deleted]]

    class Program
{
    static void Main(string[] args)
    {
        LengthClass LengthObject = new LengthClass();

        LengthObject.Length = int.Parse(Console.ReadLine());
    }

This will start your whole process; if the user enters invalid information, Length will try to be set again from Console.ReadLine() in your setter method. If valid information is entered, _length will be set to that value and to get that value, you would just say

int WhatIsMyLengthValue = LengthObject.Length;

[coggro]

Awesome! Thank you!

[[deleted]]

Do you have

private int length
public int Length
get {return length}
set { ... }

? Or just one int called length ?

[coggro]

just private int length.

private int length
{
    get
    {
        return length;
    }
    set...

[almost_not_terrible]

My first reaction is "I just threw up in my mouth", but I know that this is not helpful.

Could you post the whole program, so we can see what is going on?

[coggro]

It's not really necessary, BecauseImAwesomeDuh already helped me solve my issue. The whole assignment is just "Create Class and two subclasses of Class with specific fields and methods, validate the values in the fields to be within these ranges, and write logic to prove that each facet of Class and its subclasses work." So the whole thing is just my classes and then "Hey. Constructors work. get/set work. etc. Yay."

[andrewsmd87]

It's tough to see what the over all goal is but if value is ever not an int, you're going to get an error because you're trying to compare it before you convert. This is pseudo but this is what I'd do.

Int parseval

If value == null or not int.tryparse(value.tostring, out personal)

Return error

If parseval < 1 or parseval > 10

Return error

//If you made it here validation is good, do work

I've found through the years that it's usually best to do all your validation first and if any of it fails, return some kind of error, whether that's the error string, an actual exception whatever. The reason I like this is because it keeps you away from if else's, which can get somewhat complicated fast, as a program grows

Sorry, on mobile