CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Results 1 to 4 of 4
  1. #1
    Join Date
    May 2000
    Location
    England
    Posts
    574

    Horrible code needs code review .... Anybody ?

    Hi
    i need a better way of doing all of these if / else statements
    any ideas ?

    Code:
    	if (nCodePage == 0000) //		(obsolete!) R/3 System character set
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);
    		}
    	else if (nCodePage == 120) //		EBCDIC ISO-1 (Latin 1) 
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);
    		}
    	else if (nCodePage == 1100) //		ASCII ISO 8859/1 (Latin 1) 
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);
    		}
    	else if (nCodePage == 410) //		EBCDIC ISO-2 (East. Europe, Latin 2) 
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);
    		}
    	else if (nCodePage == 500) //		EBCDIC ISO-5 (Russian) 
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf); 
    		}
    	else if (nCodePage == 610) //		EBCDIC ISO-9 (Turkish) 
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf); 
    		}
    	else if (nCodePage == 700) //		EBCDIC ISO-7 
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);  
    		}
    	else if (nCodePage == 800) //		EBCDIC ISO-8 (Hebrew)
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);    
    		}
    	else if (nCodePage == 1400) //		ASCII ISO 8859/2 (Latin 2) 
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);  
    		}
    	else if (nCodePage == 1500) //		ASCII ISO 8859/5 (Russian)  
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);  
    		}
    	else if (nCodePage == 1600) //		ASCII ISO 8859/3  
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf); 
    		}
    	else if (nCodePage == 1610) //		ASCII ISO 8859/9 (Turkish) 
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);   
    		}
    	else if (nCodePage == 1700) //		ASCII ISO 8859/7 (Greek)
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);  
    		}
    	else if (nCodePage == 1802) //		ASCII ISO 8859/8 (Hebrew)
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);    
    		}
    	else if (nCodePage == 4001) //		OCR-A (ASCII) 
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);   
    		}
    	else if (nCodePage == 4004) //		OCR-B (ASCII) 
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);    
    		}
    	else if (nCodePage == 8000) //		Japanese ISO Shift-JIS 
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);   
    		}
    	else if (nCodePage == 1802) //		ASCII ISO 8859/8 (Hebrew)
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);    
    		}
    	else if (nCodePage == 8300) //		Chinese (traditional) 
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);  
    		}
    	else if (nCodePage == 8400) //		Chinese (simplified)  
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf); 
    		}
    	else if (nCodePage == 8500) //		Korean ISO KSC 5601 
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);    
    		}
    	else if (nCodePage == 8600) //		Thai ISO TIS620-2529  
    		{
    			m_pFiller->AddText("Input code page", strCode);
    			m_pFiller->AddText("Output code page", strOutput);
    			m_pFiller->AddText("Language key", strEndOf);  
    		}
    	else if (nCodePage == 'ISO ')
    		{
    			// ** have to choose between ** //
    
    			if (strOutput == "Big5")
    			{
    					m_pFiller->AddText("Input code page", strCode);
    					m_pFiller->AddText("Output code page", strOutput);
    					m_pFiller->AddText("Language key", strEndOf);
    			}
    			else if (strOutput == "GB23")
    			{
    					m_pFiller->AddText("Input code page", strCode);
    					m_pFiller->AddText("Output code page", strOutput);
    					m_pFiller->AddText("Language key", strEndOf);
    			}		
    		}
    Thanks in advance

    P

  2. #2
    Join Date
    Feb 2004
    Location
    Romania
    Posts
    57
    I'm a little bit confuzed because you do the same thing on every confition. Why do you need 100 conditions if in all cases you do the same thing ?

    m_pFiller->AddText("Input code page", strCode);
    m_pFiller->AddText("Output code page", strOutput);
    m_pFiller->AddText("Language key", strEndOf);

    If you still want some "refactoring" on that try using a switch/case:

    switch( nCodePage )
    {
    case 0:
    case 120:
    case 1100:
    case 410:
    case 500:
    <etc>
    case 8000:
    {
    m_pFiller->AddText("Input code page", strCode);
    m_pFiller->AddText("Output code page", strOutput);
    m_pFiller->AddText("Language key", strEndOf);
    break;
    }
    }

    For that if you will need later to do some other operations in some particular case you will just add the code to that "case".

    Good Luck.

  3. #3
    Join Date
    Oct 2001
    Location
    Dublin, Eire
    Posts
    880
    If most are doing the same, you can do as suggested and use a switch, or put all conditions into the same if statement:

    Code:
    if ((nCodePage == 0000)  || (nCodePage == 120) || ... )
    {
    }
    else if (nCodePage == 'ISO')
    {
    }
    else
    {
    }
    One or the other solutions at least avoid havind to repeat the same things each time ...

    In your case I think the switch statement looks nicer, easier to read.
    Elrond
    A chess genius is a human being who focuses vast, little-understood mental gifts and labors on an ultimately trivial human enterprise.
    -- George Steiner

  4. #4
    Join Date
    Apr 1999
    Posts
    27,449

    Re: Horrible code needs code review .... Anybody ?

    Originally posted by posty68
    Hi
    i need a better way of doing all of these if / else statements
    any ideas ?
    How can "nCodePage" be both an integer and a (fake) string?
    Code:
    if (nCodePage == 0000)
    //...
    else if (nCodePage == 'ISO ') //???
    Regards,

    Paul McKenzie

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  





Click Here to Expand Forum to Full Width

Featured