Skip to content

Peers now transmit their role before sending file#13

Open
BrunoFreitasgit wants to merge 4 commits intopsantosl:masterfrom
BrunoFreitasgit:check-roles
Open

Peers now transmit their role before sending file#13
BrunoFreitasgit wants to merge 4 commits intopsantosl:masterfrom
BrunoFreitasgit:check-roles

Conversation

@BrunoFreitasgit
Copy link
Copy Markdown

Fix #12 by transmitting roles before file transfer.

Program.cs Outdated
class Program
{
public static string ReceiverRole = "receiver";
public static string SenderRole = "sender";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's rather use an enum? and then use .ToString()

Program.cs Outdated
static int SleepTime(DateTime now)
{
List<int> seconds = new List<int>() {10, 20, 30, 40, 50, 60};
List<int> seconds = new List<int>() { 10, 20, 30, 40, 50, 60 };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's try to remove hunks that are unrelated

if (role == myRole.ToString())
{
Console.Error.WriteLine("Peers can't have the same role.");
return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if you exit the function like this, this won't translate into a non-zero exitcode for the program

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

What do you suggest instead?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

call directly Environment.Exit? I know it doesn't seem very clean, but it would be more correct.

@knocte
Copy link
Copy Markdown
Contributor

knocte commented Feb 9, 2021

@psantosl can you review/merge?

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.

Should not crash if both peers by mistake have the same role (sender or receiver) at the same time

2 participants