GridObject::ToString

Unity Courses Talk
,
#1

You can also do this to generate the labels for all the units in a given grid

string unitString = _unitList.Aggregate("", (string acc, Unit unit) => acc + unit + “\n”);

#2

Yes, you can. Very nice.

Although

  • It uses Linq which is OK in some cases and not so OK in others. In this case it’s probably fine.
  • Also uses "" which is no longer an issue, but - I believe - in the past used to create a new string every time and the recommended way was to use string.Empty which referenced the same string. But, like I said, this is no longer an issue.
  • And one of my biggest peeves; using math to concatenate strings.

None if these are wrong. Just things I personally don’t like, or may (or not) cause very slight performance issues

The way I would’ve written this would’ve been

var unitString = _unitList.Aggregate(string.Empty, (concat, unit) => $"{concat}{unit}\n");

and then - at some point - I would probably have changed it to

var unitString = string.Join("\n", _unitList);
2 Likes
#3

Originally I had

// GridObject.cs:
public override string ToString()
{  
  string gridObjectString = _gridPosition.ToString();
  foreach (Unit unit in _unitList)
  {
      gridObjectString += $"\n{unit.name}";
  }
  return gridObjectString;
}

When I use this one:

var unitString = string.Join("\n", _unitList);

instead, like this:

GridObject.cs:
public override string ToString()
{
    string gridObjectString = _gridPosition.ToString() + "\n";
    return gridObjectString + string.Join("\n", _unitList);
}

I would get “Unitname (Unit)” which I think clutters up the grid too much,

Is there a better way to just get a unit’s name than overriding the Unit's toString() like this?

// Unit.cs:
public override string ToString()
{
    return name;
}

And would using string.Join() actually be more efficient (performance itself as well as memory usage)?

#4

The string.Join in this case, where it is taking in a collection, has no direct control over what is returned, it simply uses element, which automatically defaults to element.ToString().
This means that yes, the most efficient method of getting just the name without all of the added (Unit) is to override ToString() in just the manner you’ve used.

Without deep profiling, it’s difficult to say. It’s part of the System Library, so it’s implementation is not exposed to attempts to dig into the source. Since it’s not System.Linq; it may indeed be faster than the original method, especially within the Editor, as in editor all C# scripts are run in the CLR (while native libraries like System and UnityEngine are run in native code. If you build the code into a player, and use IL2CPP, it will run faster, but here’s the tricky part… in order to use a profiler, you’ll need to run in Development mode, and in this case the System library may appear to outperform the for loop simply because the System library will never include debugging symbols. Built to IL2CPP without being a Development mode build is the fastest your code will run, but the harder it is to profile.

On balance, I would say that string.Join is probably faster than just about anything we could write.

1 Like
#5

Hi, I didnt like the clutter of “Unitname (Unit)” either so I did this in GridObject

    public override string ToString()
    {
        string unitNames = "";
        foreach (Unit unit in unitList)
        {
            string unitString = unit.ToString().Remove(unit.ToString().Length - 6);
            unitNames += unitString + "\n";   
        }
        return gridPosition.ToString() + "\n" + unitNames;
    }

Thoughts ???

#7

Hardcoding the length is bad… and you might want to drop the whitespace between the unit’s name and the “(Unit)” type part…

Since you know the part in front should be the object’s name, just go directly with it and avoid the extra string operations… :wink:

1 Like

Privacy & Terms