-1

I implemented an extension method to get fields from an Excel file (everything in .NET Framework 4.8), but the fields object is not populatedd with Field values in the Main() method, while it really has values inside the extension method. Does anyone see any issue? Am I missing an out keywork somewhere?

I'mm afraid that there are two references / pointers towards different fields objects..

Also, do I really have to instantiate fields before "applying" an extension method?

static void Main(string[] args)
{
    Fields fields = new Fields(); // Fields is a class which implements IEnumerable<Field>

    fields.ReadFromExcel(); // fields is still null here, despite having values within the extension method
}

Extension method (simplified)

public static class ExtensionMethods
{
    public static void ReadFromExcel(this Fields fields)
    {
        ExcelConnector excel = new ExcelConnector();
        
        fields = excel.ReadFields(); // fields is correctly populated with fields from Excel here
    }
}

Thanks for any insights :-)

Update

If I change to fields = Processor.ReadFromExcel(); and

public static class Processor
{
    public static Fields ReadFromExcel()
    {
        // ...
        return excel.ReadFields;
    }
}

it works well, but then loose the extension method features...

derloopkat
  • 6,232
  • 16
  • 38
  • 45
Ludovic Wagner
  • 115
  • 2
  • 8
  • Just like with any other method, modifying an input parameter does not change the value at the calling site – UnholySheep Sep 25 '20 at 21:54
  • I am finding it hard to understand what the actual question is without guessing. Are you saying the variable fields is null in main before you call ReadFromExcel, or that fields is null afterwards, or that it does not contain what you would expect – TheGeneral Sep 25 '20 at 21:57
  • No Michael, it is not `null` in the `Main()` method, but is not populated with the values retrived from Excel. It remains as an initialized `List` with no item. Is there no way to modify the calling value? By the way, I came from VBA background, I've never had the opportunity to dea with such nice features so far :-( – Ludovic Wagner Sep 25 '20 at 22:02
  • 2
    This is not a good use case for an extension method. It's a thin wrapper around `ExcelConnector`'s `ReadFields` method. As such, it should return a `Fields` instance just as `ReadFields` does, like you showed in your update. Losing the extension method feature is a *good* thing here. – madreflection Sep 25 '20 at 22:04
  • Thanks, lesson learnt! I also have similar connectors to SQLServer and possibly SQLite, MySQL, etc. also. I added a Processor class where I move the ReadFromFields with an actual return object. Yes, `ReadFromExcel()` is a wrapper with Console, WPF or ASP.NET features of the `ExcelConnector()` accessible in a shared library. – Ludovic Wagner Sep 25 '20 at 22:14

1 Answers1

2

When you do:

Fields fields = new Fields(); // Fields is a class with implements IEnumerable<Field>

fields.ReadFromExcel();

you are passing the reference that fields has to ReadFromExcel. Then when you do:

public static void ReadFromExcel(this Fields fields)
{
    fields = null;
}

you are only modifying to what the parameter fields points to. You don't change the reference of the original variable.

While you could do:

public static Fields ReadFromExcel(this Fields fields)
{
    return new ExcelConnector().ReadFields();
}

// and 

Fields fields = new Fields();
fields = fields.ReadFromExcel();

That's for the most part wasting the initial fields assignment. This is also not at all a good usage for extension methods, as you are not extending Fields but rather just creating an entirely new instance. This should be just:

public static Fields ReadFromExcel()
{
    ExcelConnector excel = new ExcelConnector();
    
    return excel.ReadFields();
}

// and

Fields fields = ExtensionMethods.ReadFromExcel();

Also, most likely you shouldn't have a Fields class, you can and should use List<Field> (or IEnumerable<Field> if you don't need to change the collection).

Camilo Terevinto
  • 31,141
  • 6
  • 88
  • 120
  • OK, I'm sure that there are ways to investigate it further with VS (actual addresses), but it is beyond my skills. Still need to think about it a bit.. So, `fields = null;` is implicit? I have to admit that despite the underlying issues, I found it rather nicely written in the `Main()` method ^^ . Sure `List` would work, but I want to add additional methods such as `SearchByDate()` (even if Linq would be as straightforward). – Ludovic Wagner Sep 25 '20 at 22:23