patterncMinor
A-Eye: Analysing Eye tracking Data
Viewed 0 times
trackinganalysingdataeye
Problem
I'd appreciate your advice.
data file: https://drive.google.com/open?id=0B_UEM0TEmA_yeHJBZ0dzbV9Ca2c
```
/*
CNG213 Data Structures / Programming Assignment 1 - Sinan ULUSOY
"A-Eye: Analysing Eye tracking Data"
l1 is used for main list record
l2 is used for sub list record
*/
#include
#include
#include
struct webNamesNode
{
char StimuliName[50];
struct webNamesNode *next;
struct detailsRecord *dummyNodeOFdetails;
};
struct detailsNode
{
struct detailsNode *next;
int FixationIndex;
int Timestamp;
int FixationDuration;
int FixationPointX;
int FixationPointY;
};
struct webNamesRecord
{
struct webNamesNode *head;
struct webNamesNode *tail;
int size;
};
struct detailsRecord
{
struct detailsNode *head;
struct detailsNode *tail;
int size;
};
//create functions////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
struct webNamesRecord *makeMainList()
{
struct webNamesRecord *l1;
l1 = (struct webNamesRecord *) malloc(sizeof(struct webNamesRecord));
if (l1 == NULL)
printf("Out of memory!\n");
l1->head = (struct webNamesNode*) malloc(sizeof(struct webNamesNode));
if (l1->head == NULL)
printf("Out of memory!\n");
l1->head->next = NULL;
l1->head->dummyNodeOFdetails = NULL;
l1->tail = l1->head;
l1->size = 0;
return l1;
}
struct detailsRecord *makeSubList()
{
struct detailsRecord *l2;
l2 = (struct detailsRecord *) malloc(sizeof(struct detailsRecord));
if(l2 == NULL)
printf("Out of memory!\n");
l2->head = (struct detailsNode *) malloc(sizeof(struct detai
data file: https://drive.google.com/open?id=0B_UEM0TEmA_yeHJBZ0dzbV9Ca2c
```
/*
CNG213 Data Structures / Programming Assignment 1 - Sinan ULUSOY
"A-Eye: Analysing Eye tracking Data"
l1 is used for main list record
l2 is used for sub list record
*/
#include
#include
#include
struct webNamesNode
{
char StimuliName[50];
struct webNamesNode *next;
struct detailsRecord *dummyNodeOFdetails;
};
struct detailsNode
{
struct detailsNode *next;
int FixationIndex;
int Timestamp;
int FixationDuration;
int FixationPointX;
int FixationPointY;
};
struct webNamesRecord
{
struct webNamesNode *head;
struct webNamesNode *tail;
int size;
};
struct detailsRecord
{
struct detailsNode *head;
struct detailsNode *tail;
int size;
};
//create functions////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
struct webNamesRecord *makeMainList()
{
struct webNamesRecord *l1;
l1 = (struct webNamesRecord *) malloc(sizeof(struct webNamesRecord));
if (l1 == NULL)
printf("Out of memory!\n");
l1->head = (struct webNamesNode*) malloc(sizeof(struct webNamesNode));
if (l1->head == NULL)
printf("Out of memory!\n");
l1->head->next = NULL;
l1->head->dummyNodeOFdetails = NULL;
l1->tail = l1->head;
l1->size = 0;
return l1;
}
struct detailsRecord *makeSubList()
{
struct detailsRecord *l2;
l2 = (struct detailsRecord *) malloc(sizeof(struct detailsRecord));
if(l2 == NULL)
printf("Out of memory!\n");
l2->head = (struct detailsNode *) malloc(sizeof(struct detai
Solution
Nice question, it deserved more attention then it got when it was posted. It might have gotten more attention if you had only posted the contents of page 2 and page 3 (the program requirements).
It's not clear to that the modular programming requirement was met, since that might have indicated that the program should have been broken up into multiple C and header files and been built as a project in visual studio, eclipse or Xcode. The struct declarations at least could have been in header files. There could have been one C file for each of the structs which exclusively manipulated that particular struct.
It might have been easier to implement and debug the program if each of the
Possible Bugs
There is a possible bug in
As a side note, the comment
A second possible future bug is that in the following code pointers are not initialized to
If either of these pointers is tested for != NULL they may fail the test.
Portability
Use Common Library Routines Whenever Possible
The code is not portable because it uses the function
Prefer powers of Two When Creating String Sizes
There are many places in the code where the a character array is defined as
It might be better to size character arrays and strings on based word size, not doing so can sometimes result in memory alignment errors. At minimum use some power of 2 as a character array / string size. One possible way to do this might be:
or alternately:
Please note that using a symbolic constant such as
Numeric constants in code are sometimes referred to as Magic Numbers.
Provide Enough Memory for a Full File Specification in a File Name or a Full Web Address
In the function
There are 2 problems with this code, the first being that numeric constants rather than symbolic constants are used which may make it difficult to to resize these variables, the second and possibly more major problem is that these simply aren't large enough to hold the possible values of a full file specification or a full web page address.
A full file specification includes the full path from the root directory to the file, including the local file name. A windows file specification might be
The same can also be true of a web page address, the page address is not guaranteed to be at the top level of the domain and generally the
question which is a web page is:
File systems differ on different operating systems. A symbolic constant such as
the maximum file name size allowed. This migh
It's not clear to that the modular programming requirement was met, since that might have indicated that the program should have been broken up into multiple C and header files and been built as a project in visual studio, eclipse or Xcode. The struct declarations at least could have been in header files. There could have been one C file for each of the structs which exclusively manipulated that particular struct.
It might have been easier to implement and debug the program if each of the
list types (webNamesNode, webNamesRecord and detailsRecord) had common list functions such as insert(node, list), add(node, list) and delete(node, list).Possible Bugs
There is a possible bug in
load_data_points(), l2 may be used before it is defined in the following code://the most important part!!!//////////////////////////////////////////////////////////////////////////
//compares web names to make node for main list and list record for sublist
if( strcmp(StimuliName, previousStimuliName) != 0)
{
makeNodeForMainList(l1, StimuliName);
l2 = makeSubList();
}
//starts to put values into sublist
makeNodeForSubList(l2, FixationIndex, Timestamp, FixationDuration, FixationPointX, FixationPointY);As a side note, the comment
//the most important part! doesn't explain why it is the most important part.A second possible future bug is that in the following code pointers are not initialized to
NULL.struct webNamesRecord *l1;
struct detailsRecord *l2;If either of these pointers is tested for != NULL they may fail the test.
Portability
Use Common Library Routines Whenever Possible
The code is not portable because it uses the function
char strlwr(char str). strlwr() is only available some operating systems and the code may not link in some cases. See the first answer to this Stack Overflow question.Prefer powers of Two When Creating String Sizes
There are many places in the code where the a character array is defined as
char StimuliName[50];
char previousStimuliName[50] = "web";It might be better to size character arrays and strings on based word size, not doing so can sometimes result in memory alignment errors. At minimum use some power of 2 as a character array / string size. One possible way to do this might be:
#define CHARS_IN_WORD sizeof(int) // gives the number of characters in an int which is the basic C size.
#define COMMON_STRING_LENGTH 80 * CHARS_IN_WORDor alternately:
#define COMMON_STRING_LENGTH 80 * sizeof(int)
char characterArray[COMMON_STRING_LENGTH];Please note that using a symbolic constant such as
COMMON_STRING_LENGTH makes it much easier in the future if the size of an array needs to change. This allows the code to be changed in only one place to change all of the arrays and change any for loops that may depend on the size. The words used might indicate what the number means as well which can help in debugging or if someone else needs to modify the code.Numeric constants in code are sometimes referred to as Magic Numbers.
Provide Enough Memory for a Full File Specification in a File Name or a Full Web Address
In the function
main() there is the following code:char fileName[20], pageName[50];There are 2 problems with this code, the first being that numeric constants rather than symbolic constants are used which may make it difficult to to resize these variables, the second and possibly more major problem is that these simply aren't large enough to hold the possible values of a full file specification or a full web page address.
A full file specification includes the full path from the root directory to the file, including the local file name. A windows file specification might be
C:\user\documents\FILENAME.EXT. A Unix/Linux file specification might be /Users/USERNAME/Documents/a-eye-analysing-eye-tracking-data/P1.txt. The file to be opened may not be in the local directory and therefore enough space for the full path should be allowed. The size of fileName could quite easily cause the program to fail to load the file.The same can also be true of a web page address, the page address is not guaranteed to be at the top level of the domain and generally the
http:// portion should be attached as well. As an example, the URL of thisquestion which is a web page is:
https://codereview.stackexchange.com/questions/146870/a-eye-analysing-eye-tracking-dataFile systems differ on different operating systems. A symbolic constant such as
PATH_MAX, MAXPATHLEN, or FILENAME_MAX can generally be found in one of the common header files (stdio.h for example) that definesthe maximum file name size allowed. This migh
Code Snippets
//the most important part!!!//////////////////////////////////////////////////////////////////////////
//compares web names to make node for main list and list record for sublist
if( strcmp(StimuliName, previousStimuliName) != 0)
{
makeNodeForMainList(l1, StimuliName);
l2 = makeSubList();
}
//starts to put values into sublist
makeNodeForSubList(l2, FixationIndex, Timestamp, FixationDuration, FixationPointX, FixationPointY);struct webNamesRecord *l1;
struct detailsRecord *l2;char StimuliName[50];
char previousStimuliName[50] = "web";#define CHARS_IN_WORD sizeof(int) // gives the number of characters in an int which is the basic C size.
#define COMMON_STRING_LENGTH 80 * CHARS_IN_WORD#define COMMON_STRING_LENGTH 80 * sizeof(int)
char characterArray[COMMON_STRING_LENGTH];Context
StackExchange Code Review Q#146870, answer score: 6
Revisions (0)
No revisions yet.