Skip to content

Conversation

@Dzejkop
Copy link
Contributor

@Dzejkop Dzejkop commented Jun 7, 2018

No description provided.

@Dzejkop Dzejkop self-assigned this Jun 7, 2018
@Dzejkop Dzejkop requested review from genail and witcher112 June 7, 2018 12:51
Copy link
Contributor

@genail genail left a comment

Choose a reason for hiding this comment

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

Please see comments

repairCost *= installedVersionContentSummary.Chunks.Size;
}

if (repairCost < contentSize)
Copy link
Contributor

Choose a reason for hiding this comment

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

repairCost seems to be a number of missing and broken files (x2). Is contentSize the content size in bytes?
This does not seem right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Below that there is:

if (isNewVersionAvailable)
{
    repairCost *= latestVersionContentSummary.Chunks.Size;
}
else
{
    repairCost *= installedVersionContentSummary.Chunks.Size;
}

repairCost is estimated to be equal to the sum of missing and broken files, multiplied by two and then multiplied by chunk size in the respective version.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I didn't notice that.
I can't imagine it though. For instance if there's a large file broken, would the cost be the same as for small file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it now. The cost calculation will now use actual file sizes.

Copy link
Contributor

@genail genail left a comment

Choose a reason for hiding this comment

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

Please answer to my comment.

repairCost *= installedVersionContentSummary.Chunks.Size;
}

if (repairCost < contentSize)
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I didn't notice that.
I can't imagine it though. For instance if there's a large file broken, would the cost be the same as for small file?

Copy link
Contributor

@genail genail left a comment

Choose a reason for hiding this comment

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

Possible NRE?

{
return filesToRepair
.Select(f => contentSummary.Files.FirstOrDefault(e => e.Path == f.FileName))
.Sum(f => f.Size);
Copy link
Contributor

Choose a reason for hiding this comment

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

Will it work? Select() transforms enumerable to a new form (https://msdn.microsoft.com/en-us/library/bb548891(v=vs.110).aspx). When a file is not found, a null is returned. Sum() is dereferencing the value so it looks like NRE for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

When a file is not found, default value of struct AppContentSummaryFile is returned. There won't be any NRE here.

@witcher112 witcher112 merged commit e68fb66 into freezed Jun 12, 2018
@witcher112 witcher112 deleted the feautre/882 branch June 12, 2018 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants