Skip to content

fix: several bugs on games/tic_tac_toe.c#771

Merged
kvedala merged 10 commits intoTheAlgorithms:masterfrom
kana800:fix_bug
Feb 10, 2021
Merged

fix: several bugs on games/tic_tac_toe.c#771
kvedala merged 10 commits intoTheAlgorithms:masterfrom
kana800:fix_bug

Conversation

@kana800
Copy link
Copy Markdown
Contributor

@kana800 kana800 commented Nov 4, 2020

Description of Change

Issue: #729
As mentioned in the issue , The piece code listed below throws the program into an loop (from line 247) if an invalid character is entered.

        else
        {
+            int n = check_placex();
-             printf("Invalid move\n");

-            printf("Enter new position : ");
-            scanf("%d", &n1);
+           placex(n);
-            placex(n1);
        }
    }
    else
    {
-        printf("Invalid move \n");
+       int n = check_placex();
-        printf("Enter new position : ");
-        scanf("%d", &n1);
+       placex(n);
-        placex(n1);
    }
}

Fixes / Changes

Added an array [dynamic] at line36 :
+ int *stored_pos = NULL;

line 47:

+  stored_pos = (int*)calloc(9,sizeof(int))
Assignment of the stored_pos array:

line 331, line 308 & line 278

example:

+ stored_pos[e] = 1;
Freed the stored_pos memory

line 75

+ free(stored_pos);
Added a new function called check_places at line237 to line256
int check_placex(){
	char input[50];
	int n1;
	while (1){
		fgets(input,49,stdin);
		if ( strlen(input) > 2 || strlen(input)  == 0){
			fprintf(stderr,"Invalid move, Enter number 1 - 9: ");
			continue;
		}
		if(sscanf(input,"%d",&n1) != 1){
			fprintf(stderr,"Invalid move, Enter number 1 - 9: ");
			continue;
		} 
		if ((stored_pos[n1-1] == 1) || (n1== 0)){
			fprintf(stderr,"Already allocated, Enter number: ");
			continue;
		}
		return n1;
	}
}	

The while-loop in the above above function will ask for an input until it gets a correct output.

  1. The first if condition checks whether the input is within the range. (strlen() is used so string.h lib is need)
  2. Second if condition checks if the value is integer
  3. Third if condition checks if the value (posistion) is already occupied.

References

Checklist

  • Added description of change
  • Added file name matches File name guidelines
  • Added tests and example, test must pass
  • Relevant documentation/comments is changed or added
  • PR title follows semantic commit guidelines
  • Search previous suggestions before making a new one, as yours may be a duplicate.
  • I acknowledge that all my contributions will be made under the project's license.

Notes:

@kana800 kana800 marked this pull request as ready for review November 4, 2020 18:15
@Panquesito7 Panquesito7 linked an issue Nov 4, 2020 that may be closed by this pull request
@Panquesito7 Panquesito7 added the bugfix Correction to existing algorithms label Nov 4, 2020
@Panquesito7 Panquesito7 changed the title fixed several bugs on tic_tac_toe fix: several bugs on games/tic_tac_toe.c Nov 4, 2020
Copy link
Copy Markdown
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for your contribution! 🎉 👍

@Panquesito7 Panquesito7 added the approved Approved; waiting for merge label Nov 4, 2020
@Panquesito7 Panquesito7 requested a review from kvedala November 4, 2020 20:06
Copy link
Copy Markdown
Collaborator

@kvedala kvedala left a comment

Choose a reason for hiding this comment

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

a simple fix for the bug would be to simply ensure that the input number is >=0 & <=9. There is no need to make so many changes.

Comment thread games/tic_tac_toe.c Outdated
* stored_pos array will store posistion of that were already allocated by the user
* calloc(9,sizeof(int)) means an array of 9 elements.
*/
int *stored_pos = NULL;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
int *stored_pos = NULL;
static unsigned char *stored_pos = NULL;
  • static to recognize the variable as local to this file.
  • unsigned char pointer to be same as line 30
    Is this even required? isnt it serving the same purpose as game_table?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can't actually remember what I have done here, but I think the purpose of stored_pos was to make sure that the values that was inserted by the user was not overwritten.

@kana800
Copy link
Copy Markdown
Contributor Author

kana800 commented Nov 25, 2020

a simple fix for the bug would be to simply ensure that the input number is >=0 & <=9. There is no need to make so many changes.

I went a bit overboard with it

@Panquesito7 Panquesito7 added the Autochecks are failing Failing CI Auto-checks label Nov 25, 2020
@kana800
Copy link
Copy Markdown
Contributor Author

kana800 commented Nov 26, 2020

@Panquesito7 any idea why the autochecks are failing ?
I read the logs and I don't get it

@kvedala
Copy link
Copy Markdown
Collaborator

kvedala commented Nov 26, 2020

a simple fix for the bug would be to simply ensure that the input number is >=0 & <=9. There is no need to make so many changes.

I went a bit overboard with it

Good, it would be better to add your new contribution as a "v2" of the program, a new file as described in the contribution guidelines while keeping the fix to this code to its minimal.

@kana800
Copy link
Copy Markdown
Contributor Author

kana800 commented Dec 6, 2020

a simple fix for the bug would be to simply ensure that the input number is >=0 & <=9. There is no need to make so many changes.

I went a bit overboard with it

Good, it would be better to add your new contribution as a "v2" of the program, a new file as described in the contribution guidelines while keeping the fix to this code to its minimal.

Kept everything to minimal, removed stored_pos and used game_table . Instead of using scanf i went with another approach (line : 230), scanf will take the \n of the input buffer.

Copy link
Copy Markdown
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Please fix clang-tidy warnings.

Comment thread games/tic_tac_toe.c Outdated
/**
* @brief Helper function of place_x,place_y function
*
* @param None
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need to add this if there are no parameters.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

okay

@kana800
Copy link
Copy Markdown
Contributor Author

kana800 commented Dec 19, 2020

@Panquesito7 please check the windows log

Copy link
Copy Markdown
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Comment thread games/tic_tac_toe.c
Comment thread games/tic_tac_toe.c
@kana800
Copy link
Copy Markdown
Contributor Author

kana800 commented Dec 20, 2020

Please see https://github.com/TheAlgorithms/C/runs/1581156179#step:4:1407.

Thanks a lot ! 🙏🏽

@Panquesito7
Copy link
Copy Markdown
Member

https://github.com/TheAlgorithms/C/runs/1583568825#step:4:1424

@kana800
Copy link
Copy Markdown
Contributor Author

kana800 commented Dec 22, 2020

2020-12-22T16:06:10.6979145Z   tic_tac_toe.c
2020-12-22T16:06:10.7562749Z C:\Program Files (x86)\Windows Kits\10\Include\10.0.17763.0\ucrt\stdio.h(378,9): warning C5105: macro expansion producing 'defined' has undefined behavior [D:\a\C\C\build\games\tic_tac_toe.vcxproj]
2020-12-22T16:06:10.7689739Z C:\Program Files (x86)\Windows Kits\10\Include\10.0.17763.0\ucrt\stdio.h(2437,9): warning C5105: macro expansion producing 'defined' has undefined behavior [D:\a\C\C\build\games\tic_tac_toe.vcxproj]
2020-12-22T16:06:10.7722359Z C:\Program Files (x86)\Windows Kits\10\Include\10.0.17763.0\ucrt\corecrt_search.h(188,5): warning C5105: macro expansion producing 'defined' has undefined behavior [D:\a\C\C\build\games\tic_tac_toe.vcxproj]
2020-12-22T16:06:10.7778834Z C:\Program Files (x86)\Windows Kits\10\Include\10.0.17763.0\ucrt\stdlib.h(79,5): warning C5105: macro expansion producing 'defined' has undefined behavior [D:\a\C\C\build\games\tic_tac_toe.vcxproj]
2020-12-22T16:06:10.7856859Z C:\Program Files (x86)\Windows Kits\10\Include\10.0.17763.0\ucrt\stdlib.h(1286,5): warning C5105: macro expansion producing 'defined' has undefined behavior [D:\a\C\C\build\games\tic_tac_toe.vcxproj]
2020-12-22T16:06:10.7904488Z C:\Program Files (x86)\Windows Kits\10\Include\10.0.17763.0\ucrt\time.h(589,5): warning C5105: macro expansion producing 'defined' has undefined behavior [D:\a\C\C\build\games\tic_tac_toe.vcxproj]
2020-12-22T16:06:10.7935445Z C:\Program Files (x86)\Windows Kits\10\Include\10.0.17763.0\ucrt\corecrt_memory.h(76,5): warning C5105: macro expansion producing 'defined' has undefined behavior [D:\a\C\C\build\games\tic_tac_toe.vcxproj]
2020-12-22T16:06:10.7984994Z C:\Program Files (x86)\Windows Kits\10\Include\10.0.17763.0\ucrt\corecrt_wstring.h(573,5): warning C5105: macro expansion producing 'defined' has undefined behavior [D:\a\C\C\build\games\tic_tac_toe.vcxproj]
2020-12-22T16:06:10.8038939Z C:\Program Files (x86)\Windows Kits\10\Include\10.0.17763.0\ucrt\string.h(531,5): warning C5105: macro expansion producing 'defined' has undefined behavior [D:\a\C\C\build\games\tic_tac_toe.vcxproj]

@Panquesito7
Fixed Errors from my end, anything wrong with windows version of LGTM analysis?

@Panquesito7
Copy link
Copy Markdown
Member

Panquesito7 commented Dec 23, 2020

anything wrong with windows version of LGTM analysis?

LGTM seems to be working fine (no errors/bugs reported).
Though I still see some Windows warnings reported by the CI.

@Panquesito7 Panquesito7 removed the approved Approved; waiting for merge label Dec 23, 2020
@Panquesito7 Panquesito7 added Changes requested enhancement New feature or request labels Dec 23, 2020
@Panquesito7
Copy link
Copy Markdown
Member

My bad! It seems it has something to do with latest commit in the repository (b2def5c).
Your code seems to be good, though. I do not see any errors reported. Only warnings, though.

@kvedala
Copy link
Copy Markdown
Collaborator

kvedala commented Feb 5, 2021

My bad! It seems it has something to do with latest commit in the repository (b2def5c).
Your code seems to be good, though. I do not see any errors reported. Only warnings, though.

The CI fix is present here: #796
Once this is merged,
The pull-requests and/or commits since Nov 29, 2020 can be rechecked by force triggering the CI checks on them, including this one

@kana800
Copy link
Copy Markdown
Contributor Author

kana800 commented Feb 8, 2021

Recheck done 👍🏽 , Everything seems to be working alright

@Panquesito7 Panquesito7 removed the Autochecks are failing Failing CI Auto-checks label Feb 8, 2021
@Panquesito7 Panquesito7 requested a review from kvedala February 8, 2021 16:55
Comment thread games/tic_tac_toe.c
{
srand( (unsigned int)time(NULL));
int l = 0;
do
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

,)•(

Comment thread games/tic_tac_toe.c
int main()
{ srand(time(NULL));
{
srand( (unsigned int)time(NULL));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

,(•):_

Comment thread games/tic_tac_toe.c
fprintf(stderr,"Invalid move, Enter number 1 - 9: ");
continue;
}
if(sscanf(input,"%d",&n1) != 1){
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

}{

@kvedala kvedala merged commit c4e80b8 into TheAlgorithms:master Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Correction to existing algorithms Changes requested enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Unhandled Exception in Tic-tack-toe-game

4 participants