Skip to content
This repository has been archived by the owner on Apr 12, 2023. It is now read-only.

IDateTimeUtility をサービス化 #224

Closed
wants to merge 3 commits into from

Conversation

Takym
Copy link
Contributor

@Takym Takym commented Jun 11, 2021

Issue 番号 / Issue ID

目的 / Purpose

  • IDateTimeUtility をサービス化し、外部からインスタンスの書き換えを行えなくしました。

破壊的変更をもたらしますか / Does this introduce a breaking change?

[x] Yes
[ ] No

Pull Request の種類 / Pull Request type

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[x] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

検証方法 / How to test

コードの入手 / Get the code

git clone https://github.com/Takym/cocoa.git
cd cocoa
git checkout refactoring/dtutil
dotnet restore

コードの検証 / Test the code

確認事項 / What to check

  • 以前と動作が変わらない。
  • テストが正しく通る。

その他 / Other information

  • IDateTimeUtilityIHttpClientService は持っている機能が少なく似ているので統合しても良いかもしれませんね。
public interface IFactoryService
{
    public DateTime UtcNow { get; }
    public HttpClient CreateHttpClient();
}

public class FactoryService : IFactoryService
{
    public DateTime UtcNow => DateTime.UtcNow;

    public HttpClient Create()
    {
        return new HttpClient();
    }
}

Internal IDs:

  • NFR 2900

@Takym Takym mentioned this pull request Jun 11, 2021
1 task
@keiji keiji self-requested a review June 11, 2021 08:37
Copy link
Collaborator

@keiji keiji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます。
ライセンス通知、完全に見落としていました。
.editorconfigでライセンス通知を付けられるならそうしたいですね。

@@ -161,6 +161,7 @@ protected override void RegisterTypes(IContainerRegistry containerRegistry)
private static void RegisterCommonTypes(IContainer container)
{
// Services
container.Register<IDateTimeUtility, DateTimeUtility>(Reuse.Singleton);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

役割と利用頻度から言うと一番下のISecureStorageServiceの下に置いた方がいいかなと思いましたが、好みの問題かも。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ISecureStorageService の下に移動し「Utilities」というコメントを付けておきました。

using System;
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あれ? ライセンス表記ありませんね。
ありがとうございます:+1:

やっぱり自動でライセンスを付けたりチェックしたりする仕組みが欲しいですね……

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ブランチ tools/header にて自動的にライセンス通知を追加してくれるツールを作りました。(#80)


public string CurrentStatusMessage { get; set; } = "初期状態";
public Status ExposureNotificationStatus { get; set; }

public ExposureNotificationService(ILoggerService loggerService, IHttpClientService httpClientService, ISecureStorageService secureStorageService, IPreferencesService preferencesService, IApplicationPropertyService applicationPropertyService)
public ExposureNotificationService(ILoggerService loggerService, IHttpClientService httpClientService, ISecureStorageService secureStorageService, IPreferencesService preferencesService, IApplicationPropertyService applicationPropertyService, IDateTimeUtility dateTimeUtility)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

これくらいコンストラクタの引数が長くなると、改行して縦に伸ばした方がいいかもしれません。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

対応しました。

@keiji
Copy link
Collaborator

keiji commented Nov 21, 2021

@Takym
ようやくですが、このPull Requestを取り込む方向で進んでいます。
developに対するコンフリクトも大きくなっている(+別のテストにも影響が波及する)ので、このPull Requestを変更せずfeatureブランチに取り込んでからぼくで解消することを考えています。もちろん、Takymさんご自身で解消していいただくのもウェルカムですので、ご希望がありましたら教えてください。

@keiji
Copy link
Collaborator

keiji commented Jan 8, 2022

@Takym
いただいているコミットは #482 を通じてdevelopブランチに取り込まれました。
ありがとうございました。

@keiji keiji closed this Jan 8, 2022
@Takym
Copy link
Contributor Author

Takym commented Jan 8, 2022

どういたしまして。

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
confirmed 開発内部管理用
Projects
None yet
3 participants