patterncMinor
Translating warning codes as messages to be printed
Viewed 0 times
translatingcodesmessageswarningprinted
Problem
I am writing an X application with Xlib in C, and this code is for error handling and error printing.
How to make it simpler and clearer without using a large amount of
How to make it simpler and clearer without using a large amount of
fprintf?int XB3DS_Warn_Code (int Warn_Code)
{
switch(Warn_Code)
{
case WARN_Failed_To_Connect_To_Display:
fprintf(stdout, "[ WARNING ]: Failed to connect to display.\n");
break;
case WARN_Width_Or_Height_Is_Unacceptable:
fprintf(stdout, "[ WARNING ]: Width or height is unacceptable.\n");
break;
case WARN_No_Visual_Is_Matched_For_Screen:
fprintf(stdout, "[ WARNING ]: No visual is matched for screen.\n");
break;
case WARN_The_Window_Mode_Is_Unacceptable:
fprintf(stdout, "[ WARNING ]: The window mode is unacceptable.\n");
break;
case WARN_Insufficient_Memory_To_Allocate:
fprintf(stdout, "[ WARNING ]: Insufficient memory to allocate.\n");
break;
}
return Warn_Code;
}Solution
Store the variable part of the message in a variable, and do the printing after the
Now the
Also, instead of
Finally, as @black pointed out, "What's the point to return what you get passed?" Better change the method signature to return
Change the rest of your code to call
switch, for example:int XB3DS_Warn_Code (int Warn_Code)
{
const char * msg;
switch(Warn_Code)
{
case WARN_Failed_To_Connect_To_Display:
msg = "Failed to connect to display.";
break;
case WARN_Width_Or_Height_Is_Unacceptable:
msg = "Width or height is unacceptable.";
break;
case WARN_No_Visual_Is_Matched_For_Screen:
msg = "No visual is matched for screen.";
break;
case WARN_The_Window_Mode_Is_Unacceptable:
msg = "The window mode is unacceptable.";
break;
case WARN_Insufficient_Memory_To_Allocate:
msg = "Insufficient memory to allocate.";
break;
default:
msg = "Unknown error.";
}
fprintf(stdout, "[ WARNING ]: %s\n", msg);
return Warn_Code;
}Now the
case statements only have the absolutely necessary code. It's the most basic kind of refactoring: extracting values to variables and filling them in a template later. Another variant would be extracting common logic to a helper function. I also added a default case that was missing: it's always good to have.Also, instead of
fprintf(stdout, ...) why not use simply printf(...) ?Finally, as @black pointed out, "What's the point to return what you get passed?" Better change the method signature to return
void. Even better, return the message instead of printing, so that the rest of your code can do whatever it needs with it, for example:const char * XB3DS_Get_Warn_Message (int Warn_Code) {
const char * msg;
// the switch ...
return msg;
}
void XB3DS_Handle_Warning (int Warn_Code) {
const char * msg = XB3DS_Get_Warn_Message(Warn_Code);
fprintf(stdout, "[ WARNING ]: %s\n", msg);
}Change the rest of your code to call
XB3DS_Handle_Warning instead of your current XB3DS_Warn_Code. Right now you just want to print to stdout. But maybe someday you will want to log to a file, or show as part of a GUI. Then you won't need to touch XB3DS_Get_Warn_Message anymore, but change the simpler XB3DS_Handle_Warning helper.Code Snippets
int XB3DS_Warn_Code (int Warn_Code)
{
const char * msg;
switch(Warn_Code)
{
case WARN_Failed_To_Connect_To_Display:
msg = "Failed to connect to display.";
break;
case WARN_Width_Or_Height_Is_Unacceptable:
msg = "Width or height is unacceptable.";
break;
case WARN_No_Visual_Is_Matched_For_Screen:
msg = "No visual is matched for screen.";
break;
case WARN_The_Window_Mode_Is_Unacceptable:
msg = "The window mode is unacceptable.";
break;
case WARN_Insufficient_Memory_To_Allocate:
msg = "Insufficient memory to allocate.";
break;
default:
msg = "Unknown error.";
}
fprintf(stdout, "[ WARNING ]: %s\n", msg);
return Warn_Code;
}const char * XB3DS_Get_Warn_Message (int Warn_Code) {
const char * msg;
// the switch ...
return msg;
}
void XB3DS_Handle_Warning (int Warn_Code) {
const char * msg = XB3DS_Get_Warn_Message(Warn_Code);
fprintf(stdout, "[ WARNING ]: %s\n", msg);
}Context
StackExchange Code Review Q#62631, answer score: 4
Revisions (0)
No revisions yet.