Can I optimize this?

Still trying to wrap my head around Swift concepts. However the syntax in Swift seems to be changing quickly and guides sometimes still propagate old conventions. What would be the best way to achieve this?


class Path {
    public DateTime Modified => File.GetLastWriteTime(Value);
}

class Compiler {
    public virtual bool Expired(Path target) => Sources.Contains(item => item.Value is Path path && path.Modified > target.Modified);
}

My attempt to translate this to Swift seems too chatty. Is it at all possible to put those into one-liners like in C#?:

public class Path {
    func modified() -> Date? {
		do {
			return try FileManager.default.attributesOfItem(atPath: self)[FileAttributeKey.modificationDate] as? Date
		} catch {
			return nil
		}
	}
}

public class Compiler {
    public func expired(_ target: Path) -> Bool {
		guard let modtarget = target.modified() else {return false}
		return sources.contains { 
			guard let modsource = ($1 as! Path).modified() else {return false}
			return modsource > modtarget
		}
	}
}

To start with, you can remove the entire do block and the return statement in modified() by using try? instead of try:

public class Path {
    func modified() -> Date? {
		try? FileManager.default.attributesOfItem(atPath: self)[FileAttributeKey.modificationDate] as? Date
	}
}

There's not much you can do for the second one though.

public class Compiler {
  public func expired(_ target: Path) -> Bool {
    guard let target = target.modified() else { return false }
    return sources.contains {
      if let source = ($1 as? Path)?.modified() {
        source > target
      } else {
        false
      }
    }
  }
}
1 Like

One-liners not necessarily better, but there are ways to make the code read better:

public class Path {
    func modified() -> Date? {
        let attributes = try? FileManager.default.attributesOfItem(atPath: self)
        return attributes?[FileAttributeKey.modificationDate] as? Date
    }
}

public class Compiler {
    public func expired(_ target: Path) -> Bool {
        guard let modtarget = target.modified() else {
            return false
        }
        return sources
            .compactMap { $1 as? Path }
            .compactMap { $0.modified() }
            .contains { $0 > modtarget }
    }
}

I also suggest to not ignore errors, it’s tempting and sometimes seems too verbose, but even in such cases like this one you might end up passing invalid path and getting an error, without knowing it, making it hard to debug.

public class Path {
    var modifiedAt: Date? {
        get throws {
            return try FileManager.default.attributesOfItem(atPath: self)[FileAttributeKey.modificationDate] as? Date
        }
    }
}

public class Compiler {
    public func isExpired(_ target: Path) -> Bool {
        do {
            guard let targetDate = try target.modifiedAt else {
                return false
            }
            return try sources
                .compactMap { $1 as? Path }
                .compactMap { try $0.modifiedAt }
                .contains { $0 > targetDate }
        } catch {
            print("Error getting modification date: \(error)")
            return false
        }
}

I’ve updated the naming to better align with design guidelines.

3 Likes

@vns thanks for the hint regarding not-to-ignore-errors. In my case however I need a simple bool response. Expired true or false. Any error would mean true. But I'll keep that in mind.