We help IT Professionals succeed at work.

Not all code paths return a value

Mutsop
Mutsop asked
on
Hi,

I have this weird error that I don't have a return value on all paths
This is my code:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace TCG.Domain
{
    abstract class Entity<T> where T : struct
    {
        protected internal virtual T Id;

        public override bool Equals(object obj)
        {
            Entity<T> other = obj as Entity<T>;
            if(other == null)
                return false;
            if (! this.GetType().Equals(other.GetType()))
                return false;
            if(Transient() || other.Transient())
                return Id.Equals(obj);
        }

        public override int GetHashCode()
        {
            if (Transient()) { return base.GetHashCode(); }
            return Id.GetHashCode();
        }

        protected virtual bool Transient()
        {
            T value = null;
            return Id.Equals(value);
        }
    }
}

Open in new window


I get this error on the Equals override
Comment
Watch Question

might want to use else ifs
public override bool Equals(object obj)
        {
            Entity<T> other = obj as Entity<T>;
            if(other == null)
                return false;
            else if (! this.GetType().Equals(other.GetType()))
                return false;
            else if(Transient() || other.Transient())
                return Id.Equals(obj);
            else
                return false;//or whatever you want the default to be
        }

Open in new window

the most important part is the last else which would act as the default should all other conditions fail.... that's why you were getting that error
Most Valuable Expert 2011
Top Expert 2015

Commented:
To expand on p_davis' comments and your particular error, think about it like this:  you have 3 if statements, and each if has its own return statement. What happens if none of your if statements are entered? This is what the compiler is complaining about: if none of the if statements are triggered, the compiler doesn't know what the function should return because you haven't defined what that something should be.

An alternative to p_davis' suggestion could be to have a trailing return statement:

public override bool Equals(object obj)
{
    Entity<T> other = obj as Entity<T>;
    if(other == null)
        return false;
    if (! this.GetType().Equals(other.GetType()))
        return false;
    if(Transient() || other.Transient())
        return Id.Equals(obj);

    return false;
}

Open in new window


...but the readability on such a code structure leaves something to be desired. I offer the above as an expansion of my description above.
AndyAinscowFreelance programmer / Consultant

Commented:
The error isn't wierd.  It means exactly what it says - it is possible (as far as the compiler knows) to leave that function without returning a bool value.  

Author

Commented:
Quite a few differences between vb.net and c#  I see....
To be honest I'm not really sure what it should be.

This class is to give out Id's to whichever class extends it. Received from someone a few months back.
Now I'm trying to translate it but found out that it's quite difficult :)

Have you any Idea what it might be?

Also in vb.net the Transient() is like so:
value as T = nothing
            return Id.Equals(value)

Open in new window


But null in c# doesnt seem to work.
"Cannot convert null to type parameter 'T' because it could be a non-nullable value type. Consider using 'default(T)' instead."
Most Valuable Expert 2011
Top Expert 2015

Commented:
You have restricted your generic type parameter to be any value type. Value types cannot be assigned null--only reference types can. The error is suggesting you do something like this:

value as T = default(T);

Open in new window

Most Valuable Expert 2011
Top Expert 2015
Commented:
Correction:

T value = default(t);

Open in new window


I crossed my VB & C#  : \
Naman GoelPrincipal Software engineer
Commented:
This is because of if condition which is without else

You can just return false as default value:

 
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace TCG.Domain
{
    abstract class Entity<T> where T : struct
    {
        protected internal virtual T Id;

        public override bool Equals(object obj)
        {
            Entity<T> other = obj as Entity<T>;
            if(other == null)
                return false;
            if (! this.GetType().Equals(other.GetType()))
                return false;
            if(Transient() || other.Transient())
                return Id.Equals(obj);
                return false;
        }

        public override int GetHashCode()
        {
            if (Transient()) { return base.GetHashCode(); }
            return Id.GetHashCode();
        }

        protected virtual bool Transient()
        {
            T value = null;
            return Id.Equals(value);
        }
    }
}

Toggle HighlightingOpen in New WindowSelect All

Open in new window


and for this :

value as T = nothing
            return Id.Equals(value)

you can use like this:

object typeCastedValue = value as T;

if(typeCastedValue == null)
{
        return Id.Equals(typeCastedValue)
}

Author

Commented:
Thanks alot, this fixed everything
Most Valuable Expert 2011
Top Expert 2015

Commented:
@naman_goel

The "as" keyword cannot be used with value types, which the where T : struct restricts T to.

Also, even though it is proper code, and it will function correctly, this:

if(Transient() || other.Transient())
    return Id.Equals(obj);
    return false;

Open in new window


is bad programming style. It is difficult to quickly glance over the code and determine that the "return false;" is the default operation based on your indentation style.
Naman GoelPrincipal Software engineer

Commented:
yes that's true @kaufmed for value type we have to use general typecasting method


 if(typeof(T).IsValueType)
    {
        return default(T);
    }
else
{
object typeCastedValue = value as T;

if(typeCastedValue == null)
{
        return Id.Equals(typeCastedValue)
}
}
Most Valuable Expert 2011
Top Expert 2015

Commented:
@naman_goel
Your new logic is sound, but pointless. Because of the where T : struct the compiler ensures that only a value type can be used for T.
Naman GoelPrincipal Software engineer

Commented:
:)... I am just pointing towards base logic of checking, I haven't seen the method scope... :(