Skip to content

Orm in One Go hints to improper usage of the using keyword #2464

@tobias-haenel

Description

@tobias-haenel

The exercise Orm in One Go seems to indicate that the using keyword might be suited for this type of resource cleanup. Many of the community solutions ended up using a similar approach to the following:

using System;
public class Orm
{
    // ...

    public void Write(string data)
    {
        using (database)
        {
            database.BeginTransaction();
            database.Write(data);
            database.EndTransaction();
        }
    }

    // ...

Should this be considered as a correct approach? The instructions suggest that the Database.Dispose() method should be called when an exception was thrown. See

  • A call toDatabase.Dispose() will clean up the database if an exception is thrown during a transaction. This will change the state of the database to Closed.

There is no indication that it should be called after each transaction. This works for this particular implementation of Database since it only resets the internal state to Database.State.Closed and that is the desired state for a new transaction. Currently this overlaps with the final "good" state of Database.EndTransaction(), but it might break in the future in a more realistic scenario. Which lead me to create this issue, as there might be a better example use case for the using keyword. And there is also no indication that the database will still accept further transactions after Dispose() has been called. Or should each Orm instance serve only one Write()/WriteSafely call?

An alternative Database API might have a method Database.CreateTransaction() that begins a transaction internally and returns a Transaction handle that implements the IDisposable interface. This handle might have a Transaction.Write(string data) method and it could end the transaction in Transaction.Dispose(). It could also rollback the transaction when an error occurred. A user of this API could do the following.

public class Orm(Database database)
{
    private readonly Database _db = database;

    public void Write(string data)
    {
        using var transaction = _db.CreateTransaction();
        transaction.Write(data);
    }

    public bool WriteSafely(string data)
    {
        try
        {
            Write(data);
            return true;
        }
        catch
        {
            return false;
        }
    }
}

Thanks for the awesome learning platform. I absolutely enjoy learning with Excercism!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions